Closed
Bug 1297572
Opened 8 years ago
Closed 8 years ago
AllChildrenIterator misses native anonymous content from the root scroll frame
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: bholley, Assigned: bholley)
References
Details
Attachments
(2 files)
2.92 KB,
patch
|
heycam
:
review+
|
Details | Diff | Splinter Review |
3.36 KB,
patch
|
bholley
:
review+
|
Details | Diff | Splinter Review |
It detects native anonymous children by QueryFrame-ing the primary frame to nsINativeAnonymousContentCreator. This generally works, but the root scroll frame is not the primary frame of the root element, and so we miss this case. This is problematic for stylo, since we restyle using the DOM rather than the frame tree, so we need our DOM iterator to handle corner cases like this. I have a patch which I'll run through try.
Assignee | ||
Comment 1•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=879f5dcad103
Assignee | ||
Comment 2•8 years ago
|
||
Attachment #8784472 -
Flags: review?(cam)
Comment 3•8 years ago
|
||
Comment on attachment 8784472 [details] [diff] [review] Handle the root scroll frame in AllChildrenIterator. v1 Review of attachment 8784472 [details] [diff] [review]: ----------------------------------------------------------------- r=me with that fixed. ::: dom/base/ChildIterator.cpp @@ +423,5 @@ > + nsIPresShell* presShell = mOriginalContent->OwnerDoc()->GetShell(); > + nsIFrame* scrollFrame = presShell ? presShell->GetRootScrollFrame() : nullptr; > + if (scrollFrame) { > + AppendAnonymousChildrenFromFrame(scrollFrame); > + } Don't we need to do this all in GetPreviousChild too?
Attachment #8784472 -
Flags: review?(cam) → review+
Assignee | ||
Comment 4•8 years ago
|
||
boxes. r=heycam The layout inspector ends up traversing into the root scroll frame in the next patch. If we don't fix this, we erroneously enter this code here: http://searchfox.org/mozilla-central/rev/44f6964ba95b8ddd8ebf70c55b34cd2323afeef4/layout/style/nsComputedDOMStyle.cpp#682 in this test: devtools/client/inspector/markup/test/browser_markup_anonymous_04.js
Attachment #8785115 -
Flags: review+
Assignee | ||
Comment 5•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b0f2c56f633
Pushed by bholley@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e6a1ddda457e Prevent HasPseudoElementData from erroneously tracking anonymous boxes. r=heycam https://hg.mozilla.org/integration/mozilla-inbound/rev/6789cc1bb12a Handle the root scroll frame in AllChildrenIterator. r=heycam
Comment 7•8 years ago
|
||
FWIW I just pushed a try run to check that we never have non-pseudo style inheriting from anon box style, except for the viewport scroll case, and I didn't get any failures. https://treeherder.mozilla.org/#/jobs?repo=try&revision=f0d6f94673ac
Assignee | ||
Comment 8•8 years ago
|
||
Thanks! When I get around to it, I'll see if I can push a patch to try to make the root scroll NAC inherit null instead of the anon boxes.
Comment 9•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e6a1ddda457e https://hg.mozilla.org/mozilla-central/rev/6789cc1bb12a
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•