Stop the tree splicing a whole page into itself - #77
Merged
Merged
Conversation
souta sent a screenshot: their figure gallery rendered inside the file tree, under three rows of directory names. Two defects, and either one alone produces it. `?ls` asks for one level of the tree. A directory holding an `index.html` is served *as* that page, and that resolution ran before the query was looked at -- so the question "list this folder" was answered with a document. Measured against the host in the screenshot: /Vault/Pinax/?ls 468 bytes of <ul> /Vault/Pinax/OpenBoundary/?ls 15511 bytes of <html>, a rendered gallery And the tree's click handler took the row, because a site is a directory and carries `data-dir="1"`. It called `preventDefault`, fetched, and inserted what came back. So the page never opened; it was pasted into the list. Now the query is answered before the index is considered, and a site row follows its link. Only its twisty looks inside -- a directory holding an index is somewhere to read, and clicking it should read it. Neither was caught by 228 tests, because `e2e/tree/` has no directory with an index.html inside it. Two tests now, and the body is what they assert rather than the status: both answers are `200 text/html`, so a status check would have passed the whole time. Removing the fix fails the first and restores it passes. Signed-off-by: Souta <shimozono-sota631@g.ecc.u-tokyo.ac.jp>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
souta sent a screenshot: their figure gallery rendered inside the file tree, under three rows of directory names. Two defects, and either alone produces it.
?lswas answered with a document?lsasks for one level of the tree. A directory holding anindex.htmlis served as that page — and that resolution ran before the query was looked at. Measured against the host in the screenshot:And the click was taken by the expander
A site is a directory, so its row carries
data-dir="1". The handler calledpreventDefault, fetched?ls, andinsertAdjacentHTML'd whatever came back. The page never opened — it was pasted into the list.After
A site row now follows its link; only its twisty looks inside. A directory holding an index is somewhere to read, and clicking it should read it.
Why 228 tests missed it
e2e/tree/has no directory with anindex.htmlinside it, so the case never arose. Two tests now, and they assert the body rather than the status — both answers are200 text/html, so a status check would have passed throughout. Neutered check: removing the fix failsasking_for_one_level_of_a_site_is_still_a_level, restoring it passes.230 tests, 63 e2e checks against a real host, fmt/clippy clean.