Closed
Bug 1449791
Opened 6 years ago
Closed 6 years ago
Remove XUL Overlays
Categories
(Core :: XUL, enhancement, P5)
Core
XUL
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: bdahl, Assigned: bdahl)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 4 obsolete files)
Remove all the platform code supporting XUL overlays.
Updated•6 years ago
|
Priority: -- → P5
Assignee | ||
Updated•6 years ago
|
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•6 years ago
|
||
Waiting to ask for review to give the thunderbird folks some more time and I still need to do one or two more passes on XULDocument for some cleanup.
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 7•6 years ago
|
||
Adding a reminder to ask bz for review when he's back.
Flags: needinfo?(bdahl)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8970745 -
Attachment is obsolete: true
Assignee | ||
Updated•6 years ago
|
Attachment #8970746 -
Attachment is obsolete: true
Assignee | ||
Updated•6 years ago
|
Attachment #8970747 -
Attachment is obsolete: true
Assignee | ||
Updated•6 years ago
|
Attachment #8970748 -
Attachment is obsolete: true
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(bdahl)
Assignee | ||
Comment 9•6 years ago
|
||
I'm not sure if you have a preferred way of reviewing code removal, but I have it more broken down in commits in https://hg.mozilla.org/try/rev/ed541d53d3474282b0de571ce62dcd5f7b8fd434. A few of the commits had work in progress, so I think it's easier to just review overall. I'm happy to do it differently if you want though.
Comment 10•6 years ago
|
||
How is this coming along? We've removed all overlays from TB in bug 1444468 and friends. ETA for this was end of May (bug 1444468 comment #12).
Comment 11•6 years ago
|
||
mozreview-review |
Comment on attachment 8970744 [details] Bug 1449791 - Remove platform support of overlays. https://reviewboard.mozilla.org/r/239482/#review261540 r=me. This is awesome. ;) ::: dom/xul/XULDocument.cpp:363 (Diff revision 2) > // which will add style sheet clones to each document. > bool loaded; > rv = proto->AwaitLoadDone(this, &loaded); > if (NS_FAILED(rv)) return rv; > > - mMasterPrototype = mCurrentPrototype = proto; > + mCurrentPrototype = proto; mCurrentPrototype should get renamed to mPrototype, because there is only one of it now, right? Followup is fine for this. ::: dom/xul/XULDocument.cpp:413 (Diff revision 2) > > NS_IF_ADDREF(*aDocListener); > return NS_OK; > } > > -// This gets invoked after a prototype for this document or one of > +// This gets invoked after a prototype for this document is fully built in the s/a/the/ ::: dom/xul/XULDocument.cpp:457 (Diff revision 2) > > rv = PrepareToWalk(); > NS_ASSERTION(NS_SUCCEEDED(rv), "unable to prepare for walk"); > if (NS_FAILED(rv)) return rv; > > if (aResumeWalk) { I suspect the whole OnPrototypeLoadDone/ResumeWalk setup can also become simpler, now that we will really only walk one thing. Again, followup. ::: dom/xul/XULDocument.cpp:1824 (Diff revision 2) > > - // Now check the chrome registry for any additional overlays. > - rv = AddChromeOverlays(); > - if (NS_FAILED(rv)) return rv; > - > // Do one-time initialization if we're preparing to walk the No if. That's the only thing we walk. ::: dom/xul/XULDocument.cpp:1929 (Diff revision 2) > > return NS_OK; > } > > nsresult > -XULDocument::InsertXULOverlayPI(const nsXULPrototypePI* aProtoPI, > +XULDocument::ResumeWalk() Boy, mozreview's diff made a hash of this.... The raw diff is much saner. ::: dom/xul/XULDocument.cpp:2021 (Diff revision 2) > - if (NS_FAILED(rv)) return rv; > } > else { > - if (mState == eState_Master) { > - // If there are no children, and we're in the > + // If there are no children, and we're in the > - // master document, do post-order document hookup > + // master document, do post-order document hookup There is no "master document" here... just one document.
Attachment #8970744 -
Flags: review?(bzbarsky) → review+
Comment hidden (mozreview-request) |
Comment 13•6 years ago
|
||
Pushed by bdahl@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5f50dd0905ec Remove platform support of overlays. r=bz
Comment 14•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5f50dd0905ec
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Assignee | ||
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•