Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upDon't re-resolve already-resolved generated content #9969
Conversation
This fixes #7846, a failure in the "quotes-036.htm" test. Servo lays out this test correctly in its initial layout, but then messes it up in any relayout (whether it's an incremental or full layout). The problem is that the ResolveGeneratedContent traversal is not safe to run more than once on the same flow. It mutates some GeneratedContent fragments into ScannedText fragments, but leaves others unmodified (in particular, those that generate empty content). The next time layout runs, these remaining GeneratedContent fragments are processed *again* but with an incorrect correct quote nesting level (because some of the surrounding GeneratedContent fragments are gone). This patch ensures that each GeneratedContent fragment is resolved only once.
|
Is there a case where we might need to rerun this even if the flow isn't recreated? Or is it always the case that this needs to be run only once, and any damage will damage its flow? |
Any damage to generated content or text fragments causes us to fail incremental layout for the containing flow and instead reconstruct the flow from scratch. So I believe incremental changes to flows containing generated content are handled correctly (though slowly). It's possible that there are cases where changes to one flow affect the quote nesting level of its sibling flows. We might need to use a flag similar to |
|
\o/ |
|
@bors-servo: r+ |
|
|
Don't re-resolve already-resolved generated content This fixes #7846, a failure in the "quotes-036.htm" test. Servo lays out this test correctly in its initial layout, but then messes it up in any relayout (whether it's an incremental or full layout). The problem is that the ResolveGeneratedContent traversal is not safe to run more than once on the same flow. It mutates some GeneratedContent fragments into ScannedText fragments, but leaves others unmodified (in particular, those that generate empty content). The next time layout runs, these remaining GeneratedContent fragments are processed *again* but with an incorrect correct quote nesting level (because some of the surrounding GeneratedContent fragments are gone). This patch ensures that each GeneratedContent fragment is resolved only once. r? @pcwalton <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9969) <!-- Reviewable:end -->
|
|
mbrubeck commentedMar 11, 2016
This fixes #7846, a failure in the "quotes-036.htm" test. Servo lays out this test correctly in its initial layout, but then messes it up in any relayout (whether it's an incremental or full layout).
The problem is that the ResolveGeneratedContent traversal is not safe to run more than once on the same flow. It mutates some GeneratedContent fragments into ScannedText fragments, but leaves others unmodified (in particular, those that generate empty content). The next time layout runs, these remaining GeneratedContent fragments are processed again but with an incorrect correct quote nesting level (because some of the surrounding GeneratedContent fragments are gone).
This patch ensures that each GeneratedContent fragment is resolved only once.
r? @pcwalton
This change is