Support new page nav attribute changes made by SA #1221
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The original scrapePageDropdown method is still in place as a fallback, but the new data atributes will be used first. See: https://forums.somethingawful.com/showthread.php?threadid=3944917&pagenumber=3#post548658773 - The tests using the updated showthread2.html fixture (testIgnoredPost and testWeirdSizeTags) are using the new data attributes: [PageNav] Using data attributes: page 17/18 - The tests with older fixtures are falling back to the select menu method: [PageNav] Using select menu fallback: page X/Y Changes Made: 1. Updated scrapePageDropdown function (AwfulCore/Sources/AwfulCore/Scraping/Helpers.swift:144-169): - First attempts to read the new data-current-page and data-total-pages attributes - Falls back to the old select menu counting method for backward compatibility - Added logging to show which method is being used: [PageNav] Using data attributes or [PageNav] Using select menu fallback 2. Added new PageNavigationData struct and scrapePageNavigationData function (Helpers.swift:111-142): - Captures all four new data attributes: current-page, total-pages, base-url, and per-page - Available for future use when you need the base URL or per-page values 3. Updated test fixture (showthread2.html): - Replaced this file with a modern sample (taken from the same thread and roughly the same page) - Tests confirm the new attributes are being read correctly Test Results: - All 60 tests pass successfully - Logging shows the new data attributes are being used when present - Fallback to select menu works for older fixtures without the attributes The implementation ensures the app won't break when the site changes the inner HTML of the pages div, as warned by the admin.
nolanw
reviewed
Sep 28, 2025
Member
nolanw
left a comment
nolanw
left a comment
There was a problem hiding this comment.
couple nits, ok by me if you wanna merge as is though
| let baseURL = pageDiv["data-base-url"] | ||
| let perPage = pageDiv["data-per-page"].flatMap { Int($0) } | ||
|
|
||
| print("[PageNav] Using data attributes: page \(currentPage)/\(totalPages)") |
Member
There was a problem hiding this comment.
is this helpful once it's all working?
Contributor
Author
There was a problem hiding this comment.
I've removed the print but will keep returning baseURL and perPage, even though they aren't currently used for anything. They could be useful and the data is there, so might as well return it.
| let totalPages = Int(totalPagesStr) { | ||
| print("[PageNav] Using data attributes: page \(currentPage)/\(totalPages)") | ||
| return (pageNumber: currentPage, pageCount: totalPages) | ||
| } |
Member
There was a problem hiding this comment.
this looks a bit redundant with scrapePageNavigationData, is there a sensible way to call one from the other? It's ok if not, I'm not opposed to some redundancy if it makes likely changes easier.
Contributor
Author
There was a problem hiding this comment.
Removed the fallback method. Was a "just in case" idea that isn't really needed
…olely use the new scrapePageNavigationData()
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The original scrapePageDropdown method is still in place as a fallback, but the new data atributes will be used first.
See: https://forums.somethingawful.com/showthread.php?threadid=3944917&pagenumber=3#post548658773
Changes Made:
- First attempts to read the new data-current-page and data-total-pages attributes
- Falls back to the old select menu counting method for backward compatibility
- Added logging to show which method is being used: [PageNav] Using data attributes or [PageNav] Using select menu fallback
- Captures all four new data attributes: current-page, total-pages, base-url, and per-page
- Available for future use when you need the base URL or per-page values
- Replaced this file with a modern sample (taken from the same thread and roughly the same page)
- Tests confirm the new attributes are being read correctly
Test Results:
The implementation ensures the app won't break when the site changes the inner HTML of the pages div, as warned by the admin.