Closed
Bug 1053986
Opened 10 years ago
Closed 8 years ago
rename XUL layout methods on nsIFrame to include XUL in the name
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: dbaron, Assigned: dbaron)
References
(Blocks 1 open bug)
Details
Attachments
(34 files)
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dholbert
:
review+
|
Details |
We should rename the XUL layout methods on nsIFrame so that they include XUL in the name, so that people can easily distinguish the XUL-layout-specific methods from the normal methods. This was discussed in: https://groups.google.com/d/topic/mozilla.dev.tech.layout/eRVMq2jMULs/discussion
Assignee | ||
Comment 1•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=429d3411c49f
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. But with additional manual changes in accessible/. Review commit: https://reviewboard.mozilla.org/r/47767/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47767/
Attachment #8743448 -
Flags: review?(dholbert)
Attachment #8743449 -
Flags: review?(dholbert)
Attachment #8743450 -
Flags: review?(dholbert)
Attachment #8743451 -
Flags: review?(dholbert)
Attachment #8743452 -
Flags: review?(dholbert)
Attachment #8743453 -
Flags: review?(dholbert)
Attachment #8743454 -
Flags: review?(dholbert)
Attachment #8743455 -
Flags: review?(dholbert)
Attachment #8743456 -
Flags: review?(dholbert)
Attachment #8743457 -
Flags: review?(dholbert)
Attachment #8743458 -
Flags: review?(dholbert)
Attachment #8743459 -
Flags: review?(dholbert)
Attachment #8743460 -
Flags: review?(dholbert)
Attachment #8743461 -
Flags: review?(dholbert)
Attachment #8743462 -
Flags: review?(dholbert)
Attachment #8743463 -
Flags: review?(dholbert)
Attachment #8743464 -
Flags: review?(dholbert)
Attachment #8743465 -
Flags: review?(dholbert)
Attachment #8743466 -
Flags: review?(dholbert)
Attachment #8743467 -
Flags: review?(dholbert)
Attachment #8743468 -
Flags: review?(dholbert)
Attachment #8743469 -
Flags: review?(dholbert)
Attachment #8743470 -
Flags: review?(dholbert)
Attachment #8743471 -
Flags: review?(dholbert)
Attachment #8743472 -
Flags: review?(dholbert)
Attachment #8743473 -
Flags: review?(dholbert)
Attachment #8743474 -
Flags: review?(dholbert)
Attachment #8743475 -
Flags: review?(dholbert)
Attachment #8743476 -
Flags: review?(dholbert)
Attachment #8743477 -
Flags: review?(dholbert)
Attachment #8743478 -
Flags: review?(dholbert)
Attachment #8743479 -
Flags: review?(dholbert)
Attachment #8743480 -
Flags: review?(dholbert)
Attachment #8743481 -
Flags: review?(dholbert)
Assignee | ||
Comment 3•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47769/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47769/
Assignee | ||
Comment 4•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47771/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47771/
Assignee | ||
Comment 5•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47773/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47773/
Assignee | ||
Comment 6•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47775/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47775/
Assignee | ||
Comment 7•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47777/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47777/
Assignee | ||
Comment 8•8 years ago
|
||
This is a manual subset of changes written with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47779/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47779/
Assignee | ||
Comment 9•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47781/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47781/
Assignee | ||
Comment 10•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47783/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47783/
Assignee | ||
Comment 11•8 years ago
|
||
This is a manual subset of changes written with sed, over .h and .cpp files in layout/. There is also a Selection method called IsCollapsed. Review commit: https://reviewboard.mozilla.org/r/47785/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47785/
Assignee | ||
Comment 12•8 years ago
|
||
This is a manual subset of changes written with sed, over .h and .cpp files in layout/, with additional manual indentation fixes. Review commit: https://reviewboard.mozilla.org/r/47787/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47787/
Assignee | ||
Comment 13•8 years ago
|
||
This is a manual subset of changes written with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47789/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47789/
Assignee | ||
Comment 14•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47791/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47791/
Assignee | ||
Comment 15•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47793/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47793/
Assignee | ||
Comment 16•8 years ago
|
||
This is a manual subset of changes written with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47795/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47795/
Assignee | ||
Comment 17•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47797/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47797/
Assignee | ||
Comment 18•8 years ago
|
||
This is a manual subset of changes written with sed, over .h and .cpp files in layout/. It also includes manual changes in accessible/. Review commit: https://reviewboard.mozilla.org/r/47799/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47799/
Assignee | ||
Comment 19•8 years ago
|
||
This fixes some of the substance of the comment as well. Review commit: https://reviewboard.mozilla.org/r/47801/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47801/
Assignee | ||
Comment 20•8 years ago
|
||
This is a manual subset of changes written with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47803/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47803/
Assignee | ||
Comment 21•8 years ago
|
||
This is a manual subset of changes written with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47805/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47805/
Assignee | ||
Comment 22•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47807/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47807/
Assignee | ||
Comment 23•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47809/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47809/
Assignee | ||
Comment 24•8 years ago
|
||
This is a manual subset of changes written with sed, over .h and .cpp files in layout/. There is also a Selection method called IsCollapsed. Review commit: https://reviewboard.mozilla.org/r/47811/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47811/
Assignee | ||
Comment 25•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47813/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47813/
Assignee | ||
Comment 26•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47815/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47815/
Assignee | ||
Comment 27•8 years ago
|
||
This is a manual subset of changes written with sed, over .h and .cpp files in layout/. This also renames a static method on nsSprocketLayout. Note that nsFlexContainerFrame and nsRangeFrame also have IsHorizontal methods that are not renamed here, but this can be found to be relatively safe because none of the IsHorizontal methods are virtual. Review commit: https://reviewboard.mozilla.org/r/47817/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47817/
Assignee | ||
Comment 28•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47819/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47819/
Assignee | ||
Comment 29•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/, except for the removal of one comment change in nsSliderFrame.cpp. Review commit: https://reviewboard.mozilla.org/r/47821/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47821/
Assignee | ||
Comment 30•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47823/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47823/
Assignee | ||
Comment 31•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47825/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47825/
Assignee | ||
Comment 32•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47827/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47827/
Assignee | ||
Comment 33•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47829/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47829/
Assignee | ||
Comment 34•8 years ago
|
||
Written purely with sed, over .h and .cpp files in layout/. Review commit: https://reviewboard.mozilla.org/r/47831/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47831/
Assignee | ||
Comment 35•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/47833/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47833/
Comment 36•8 years ago
|
||
Comment on attachment 8743448 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::IsBoxFrame to IsXULBoxFrame. r?dholbert https://reviewboard.mozilla.org/r/47767/#review44597 Ship It!
Attachment #8743448 -
Flags: review?(dholbert) → review+
Comment 37•8 years ago
|
||
Comment on attachment 8743449 [details] MozReview Request: Bug 1053986 - Rename nsFrame.cpp static method IsBoxWrapped to IsXULBoxWrapped. r?dholbert https://reviewboard.mozilla.org/r/47769/#review44599 Ship It!
Attachment #8743449 -
Flags: review?(dholbert) → review+
Comment 38•8 years ago
|
||
Comment on attachment 8743450 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::GetMinSize to GetXULMinSize, and related methods. r?dholbert https://reviewboard.mozilla.org/r/47771/#review44601 Ship It!
Attachment #8743450 -
Flags: review?(dholbert) → review+
Comment 39•8 years ago
|
||
Comment on attachment 8743451 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::GetPrefSize to GetXULPrefSize, and related methods. r?dholbert https://reviewboard.mozilla.org/r/47773/#review44603 Ship It!
Attachment #8743451 -
Flags: review?(dholbert) → review+
Comment 40•8 years ago
|
||
Comment on attachment 8743452 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::GetMaxSize to GetXULMaxSize, and related methods. r?dholbert https://reviewboard.mozilla.org/r/47775/#review44605 Ship It!
Attachment #8743452 -
Flags: review?(dholbert) → review+
Comment 41•8 years ago
|
||
Comment on attachment 8743453 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::GetMinSizeForScrollArea to GetXULMinSizeForScrollArea. r?dholbert https://reviewboard.mozilla.org/r/47777/#review44607 Ship It!
Attachment #8743453 -
Flags: review?(dholbert) → review+
Comment 42•8 years ago
|
||
Comment on attachment 8743454 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::GetOrdinal to GetXULOrdinal. r?dholbert https://reviewboard.mozilla.org/r/47779/#review44609 Ship It!
Attachment #8743454 -
Flags: review?(dholbert) → review+
Comment 43•8 years ago
|
||
Comment on attachment 8743455 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::GetFlex to GetXULFlex. r?dholbert https://reviewboard.mozilla.org/r/47781/#review44611 Ship It!
Attachment #8743455 -
Flags: review?(dholbert) → review+
Comment 44•8 years ago
|
||
Comment on attachment 8743456 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::GetBoxAscent to GetXULBoxAscent. r?dholbert https://reviewboard.mozilla.org/r/47783/#review44613 Ship It!
Attachment #8743456 -
Flags: review?(dholbert) → review+
Comment 45•8 years ago
|
||
Comment on attachment 8743457 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::IsCollapsed to IsXULCollapsed, and related methods. r?dholbert https://reviewboard.mozilla.org/r/47785/#review44615 Ship It!
Attachment #8743457 -
Flags: review?(dholbert) → review+
Comment 46•8 years ago
|
||
Comment on attachment 8743458 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::SetBounds to SetXULBounds. r?dholbert https://reviewboard.mozilla.org/r/47787/#review44617 r=me -- thanks for keeping the indentation aligned here as well, though I noticed 2 spots you missed: ::: layout/xul/tree/nsTreeBodyFrame.cpp:408 (Diff revision 1) > } > } > > void > -nsTreeBodyFrame::SetBounds(nsBoxLayoutState& aBoxLayoutState, const nsRect& aRect, > +nsTreeBodyFrame::SetXULBounds(nsBoxLayoutState& aBoxLayoutState, const nsRect& aRect, > bool aRemoveOverflowArea) The previous line needs an indentation-fix. ::: layout/xul/tree/nsTreeColFrame.cpp:148 (Diff revision 1) > } > > void > -nsTreeColFrame::SetBounds(nsBoxLayoutState& aBoxLayoutState, > +nsTreeColFrame::SetXULBounds(nsBoxLayoutState& aBoxLayoutState, > const nsRect& aRect, > bool aRemoveOverflowArea) { The previous 2 lines need an indentation fix.
Attachment #8743458 -
Flags: review?(dholbert) → review+
Comment 47•8 years ago
|
||
Comment on attachment 8743459 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::Layout to XULLayout, and related methods with the same name. r?dholbert https://reviewboard.mozilla.org/r/47789/#review44621 r=me, just one thought (for a possible new patch): ::: layout/xul/nsBox.cpp:125 (Diff revision 1) > nsBox::BeginLayout(nsBoxLayoutState& aState) > { > #ifdef DEBUG_LAYOUT > > nsBoxAddIndents(); > - printf("Layout: "); > + printf("XULLayout: "); Perhaps we should name BeginLayout (the function where this printf lives) to "BeginXULLayout"? Because: (1) It only exists to be a helper for the beginning of the method that's now being renamed to "XULLayout()". (2) It contains this printf about "XULLayout". So for consistency on those two points, it seems like this function "BeginLayout" merits a rename to "BeginXULLayout" (in a separate patch of course). Same goes for nsBox::EndLayout().
Attachment #8743459 -
Flags: review?(dholbert) → review+
Comment 48•8 years ago
|
||
Comment on attachment 8743460 [details] MozReview Request: Bug 1053986 - Rename nsBox::GetChildBox to GetChildXULBox. r?dholbert https://reviewboard.mozilla.org/r/47791/#review44623 Ship It!
Attachment #8743460 -
Flags: review?(dholbert) → review+
Updated•8 years ago
|
Attachment #8743461 -
Flags: review?(dholbert) → review+
Comment 49•8 years ago
|
||
Comment on attachment 8743461 [details] MozReview Request: Bug 1053986 - Rename nsBox::GetNextBox to GetNextXULBox. r?dholbert https://reviewboard.mozilla.org/r/47793/#review44625 Ship It!
Updated•8 years ago
|
Attachment #8743462 -
Flags: review?(dholbert) → review+
Comment 50•8 years ago
|
||
Comment on attachment 8743462 [details] MozReview Request: Bug 1053986 - Rename nsBox::GetParentBox to GetParentXULBox. r?dholbert https://reviewboard.mozilla.org/r/47795/#review44627 Ship It!
Comment 51•8 years ago
|
||
Comment on attachment 8743463 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::GetBorderAndPadding to GetXULBorderAndPadding. r?dholbert https://reviewboard.mozilla.org/r/47797/#review44669 Ship It!
Attachment #8743463 -
Flags: review?(dholbert) → review+
Comment 52•8 years ago
|
||
Comment on attachment 8743464 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::GetBorder to GetXULBorder. r?dholbert https://reviewboard.mozilla.org/r/47799/#review44671 Ship It!
Attachment #8743464 -
Flags: review?(dholbert) → review+
Comment 53•8 years ago
|
||
Comment on attachment 8743465 [details] MozReview Request: Bug 1053986 - Fix comment referring to a GetBorder method that no longer exists. r?dholbert https://reviewboard.mozilla.org/r/47801/#review44673 ::: layout/generic/nsIFrame.h:971 (Diff revision 1) > * Return the distance between the border edge of the frame (which is > * its rect) and the padding edge of the frame. Like GetRect(), returns > * the dimensions as of the most recent reflow. > * > - * Note that this differs from StyleBorder()->GetBorder() in that > - * this describes region of the frame's box, and > + * Note that this differs from StyleBorder()->GetComputedBorder() in > + * that this describes region of the frame's box, and While you're rewriting this comment: I think there's a word missing here, in "this describes region". (It's missing an "a" or "the", I think?)
Attachment #8743465 -
Flags: review?(dholbert)
Comment 54•8 years ago
|
||
Comment on attachment 8743465 [details] MozReview Request: Bug 1053986 - Fix comment referring to a GetBorder method that no longer exists. r?dholbert https://reviewboard.mozilla.org/r/47801/#review44679 (Sorry, forgot to tick the "Ship It" checkbox. r=me)
Attachment #8743465 -
Flags: review+
Comment 55•8 years ago
|
||
Comment on attachment 8743466 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::GetPadding to GetXULPadding. r?dholbert https://reviewboard.mozilla.org/r/47803/#review44681 Ship It!
Attachment #8743466 -
Flags: review?(dholbert) → review+
Updated•8 years ago
|
Attachment #8743467 -
Flags: review?(dholbert) → review+
Comment 56•8 years ago
|
||
Comment on attachment 8743467 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::GetMargin to GetXULMargin. r?dholbert https://reviewboard.mozilla.org/r/47805/#review44683 Ship It!
Comment 57•8 years ago
|
||
Comment on attachment 8743468 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::SetLayoutManager to SetXULLayoutManager. r?dholbert https://reviewboard.mozilla.org/r/47807/#review44685 Ship It!
Attachment #8743468 -
Flags: review?(dholbert) → review+
Updated•8 years ago
|
Attachment #8743469 -
Flags: review?(dholbert) → review+
Comment 58•8 years ago
|
||
Comment on attachment 8743469 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::GetLayoutManager to GetXULLayoutManager. r?dholbert https://reviewboard.mozilla.org/r/47809/#review44687 Ship It!
Comment 59•8 years ago
|
||
Comment on attachment 8743470 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::GetClientRect to GetXULClientRect. r?dholbert https://reviewboard.mozilla.org/r/47811/#review44691 One nit on the extended commit message for this GetClientRect patch: > This is a manual subset of changes written with sed, over .h and .cpp > files in layout/. There is also a Selection method called IsCollapsed. I think that afterthought about Selection / IsCollapsed might be stale -- at least, I don't see any occurrence of "Selection" or "IsCollapsed" at all in this patch.
Attachment #8743470 -
Flags: review?(dholbert) → review+
Updated•8 years ago
|
Attachment #8743471 -
Flags: review?(dholbert) → review+
Comment 60•8 years ago
|
||
Comment on attachment 8743471 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::GetVAlign to GetXULVAlign. r?dholbert https://reviewboard.mozilla.org/r/47813/#review44693 Ship It!
Comment 61•8 years ago
|
||
Comment on attachment 8743472 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::GetHAlign to GetXULHAlign. r?dholbert https://reviewboard.mozilla.org/r/47815/#review44695 Ship It!
Attachment #8743472 -
Flags: review?(dholbert) → review+
Comment 62•8 years ago
|
||
Comment on attachment 8743473 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::IsHorizontal to IsXULHorizontal, and related methods. r?dholbert https://reviewboard.mozilla.org/r/47817/#review44701 r=me, just 2 indentation nits: ::: layout/xul/nsSliderFrame.cpp:972 (Diff revision 1) > AsyncDragMetrics dragMetrics(scrollTargetId, presShellId, inputblockId, > NSAppUnitsToIntPixels(mDragStart, > float(AppUnitsPerCSSPixel())), > sliderTrackCSS, > - IsHorizontal() ? AsyncDragMetrics::HORIZONTAL : > + IsXULHorizontal() ? AsyncDragMetrics::HORIZONTAL : > AsyncDragMetrics::VERTICAL); Nit: this line needs 3 new spaces to keep the indentation aligned. ::: layout/xul/nsSliderFrame.cpp:1276 (Diff revision 1) > nsPoint eventPoint; > if (!GetEventPoint(aEvent, eventPoint)) { > return NS_OK; > } > - if (IsHorizontal() ? eventPoint.x < thumbRect.x > + if (IsXULHorizontal() ? eventPoint.x < thumbRect.x > : eventPoint.y < thumbRect.y) Nit: this line needs 3 new spaces to keep the indentation aligned.
Attachment #8743473 -
Flags: review?(dholbert) → review+
Comment 63•8 years ago
|
||
Comment on attachment 8743474 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::IsNormalDirection to IsXULNormalDirection. r?dholbert https://reviewboard.mozilla.org/r/47819/#review44703 Ship It!
Attachment #8743474 -
Flags: review?(dholbert) → review+
Updated•8 years ago
|
Attachment #8743475 -
Flags: review?(dholbert) → review+
Comment 64•8 years ago
|
||
Comment on attachment 8743475 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::Redraw to XULRedraw. r?dholbert https://reviewboard.mozilla.org/r/47821/#review44705 Ship It!
Comment 65•8 years ago
|
||
Comment on attachment 8743476 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::RelayoutChildAtOrdinal to XULRelayoutChildAtOrdinal. r?dholbert https://reviewboard.mozilla.org/r/47823/#review44707 Ship It!
Attachment #8743476 -
Flags: review?(dholbert) → review+
Comment 66•8 years ago
|
||
Comment on attachment 8743477 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::SetDebug to SetXULDebug. r?dholbert https://reviewboard.mozilla.org/r/47825/#review44709 Ship It!
Attachment #8743477 -
Flags: review?(dholbert) → review+
Comment 67•8 years ago
|
||
Comment on attachment 8743478 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::GetDebug to GetXULDebug. r?dholbert https://reviewboard.mozilla.org/r/47827/#review44711 Ship It!
Attachment #8743478 -
Flags: review?(dholbert) → review+
Comment 68•8 years ago
|
||
Comment on attachment 8743479 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::DumpBox to XULDumpBox. r?dholbert https://reviewboard.mozilla.org/r/47829/#review44713 Ship It!
Attachment #8743479 -
Flags: review?(dholbert) → review+
Updated•8 years ago
|
Attachment #8743480 -
Flags: review?(dholbert) → review+
Comment 69•8 years ago
|
||
Comment on attachment 8743480 [details] MozReview Request: Bug 1053986 - Rename nsIFrame::AddCSSPrefSize, AddCSSMinSize, AddCSSMaxSize, and AddCSSFlex by replacing CSS with XUL. r?dholbert https://reviewboard.mozilla.org/r/47831/#review44717 Ship It!
Comment 70•8 years ago
|
||
Comment on attachment 8743481 [details] MozReview Request: Bug 1053986 - Fix ordering of methods in nsIFrame.h r?dholbert https://reviewboard.mozilla.org/r/47833/#review44721 Commit message nit: > Fix ordering of methods in nsIFrame.h r?dholbert This is a bit vague. Might be worth adding "to group XUL-related methods together" at the end. But, not a big deal.
Attachment #8743481 -
Flags: review?(dholbert) → review+
Comment 71•8 years ago
|
||
FYI, if it makes it easier for working through review notes: I opened "MozReview issues" for each of my suggestions here (visible as numbers in yellow boxes at https://reviewboard.mozilla.org/r/47833/ ), with the exception of the commit-message nits (comment 59 & comment 70) -- MozReview didn't let me open issues for those.
Assignee | ||
Comment 72•8 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #47) > Perhaps we should name BeginLayout (the function where this printf lives) to > "BeginXULLayout"? > Same goes for nsBox::EndLayout(). Ah, yes. And the same for DoLayout, actually. I meant to go back and add patches for those; I'll do all 3. (In reply to Daniel Holbert [:dholbert] from comment #59) > > This is a manual subset of changes written with sed, over .h and .cpp > > files in layout/. There is also a Selection method called IsCollapsed. > > I think that afterthought about Selection / IsCollapsed might be stale -- at > least, I don't see any occurrence of "Selection" or "IsCollapsed" at all in > this patch. What I meant was: It's a subset because there is also a Selection method called IsCollapsed, which is not changed here. (I'll change the comment to say that.)
Assignee | ||
Comment 73•8 years ago
|
||
(In reply to David Baron [:dbaron] ⌚️UTC-7 from comment #72) > What I meant was: > > It's a subset because there is also a Selection method called IsCollapsed, > which is not changed here. > > (I'll change the comment to say that.) I now realize you were pointing out that I incorrectly copied the comment to a second patch, which I'll fix.
Assignee | ||
Comment 74•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f81e6e5226fbc7b0bbe19711c219177feca4d8dc Bug 1053986 - Rename nsIFrame::IsBoxFrame to IsXULBoxFrame. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/d4317b74afed0cc795f000fa081954f2768e4638 Bug 1053986 - Rename nsFrame.cpp static method IsBoxWrapped to IsXULBoxWrapped. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/8024a1e1ec3f8478c1284e2ff8a41a089f70c6a7 Bug 1053986 - Rename nsIFrame::GetMinSize to GetXULMinSize, and related methods. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/0e8929a206ca948d21bb7f222fb8709dbe8d66f7 Bug 1053986 - Rename nsIFrame::GetPrefSize to GetXULPrefSize, and related methods. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/e84de27f354c29195e950f42d6c87649691281a2 Bug 1053986 - Rename nsIFrame::GetMaxSize to GetXULMaxSize, and related methods. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/d57a5842031930bb98f1d92c0baeedfeb7dd3e15 Bug 1053986 - Rename nsIFrame::GetMinSizeForScrollArea to GetXULMinSizeForScrollArea. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/4bddc023dac921f79c2a5e6f030781fe438544ba Bug 1053986 - Rename nsIFrame::GetOrdinal to GetXULOrdinal. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/ec597dfc7111d50c081640c9bfcf1860ea3ad9f0 Bug 1053986 - Rename nsIFrame::GetFlex to GetXULFlex. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/f054013fa44535991e42692879d8986335958880 Bug 1053986 - Rename nsIFrame::GetBoxAscent to GetXULBoxAscent. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/2942f0615e195c937394ea1a1487a792d9c9a11e Bug 1053986 - Rename nsIFrame::IsCollapsed to IsXULCollapsed, and related methods. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/932eaf157f7f0e7487ba0e67d8687c29511b1933 Bug 1053986 - Rename nsIFrame::SetBounds to SetXULBounds. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/0f17a800a22ee47486b4687e9ae482fe413ac6d4 Bug 1053986 - Rename nsIFrame::Layout to XULLayout, and related methods with the same name. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/cbba01cf1942a0cd1e3ec3e538992be7fa008634 Bug 1053986 - Rename nsBox::GetChildBox to GetChildXULBox. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/3668e6c38db183adf248a38b60c27a1fe6966f6f Bug 1053986 - Rename nsBox::GetNextBox to GetNextXULBox. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/42a916b000f111c68310852a5c38f828cdcf034f Bug 1053986 - Rename nsBox::GetParentBox to GetParentXULBox. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/13d0946a3e531eb3f238ebe9662997cd0a3d4b33 Bug 1053986 - Rename nsIFrame::GetBorderAndPadding to GetXULBorderAndPadding. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/fa4af7a37ad19cc5c5afd35586bd14f32201e937 Bug 1053986 - Rename nsIFrame::GetBorder to GetXULBorder. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/98b9e9971185f5399824214495b5d3fde16a9e66 Bug 1053986 - Fix comment referring to a GetBorder method that no longer exists. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/a92803ca0499510761fecffaf3fce035a8ccbc0f Bug 1053986 - Rename nsIFrame::GetPadding to GetXULPadding. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/bd142212e00e438b2773306b0adce945dfb81dd1 Bug 1053986 - Rename nsIFrame::GetMargin to GetXULMargin. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/e81b89fbb11277cb79eca66a9017635d5f250fc0 Bug 1053986 - Rename nsIFrame::SetLayoutManager to SetXULLayoutManager. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/9e1500799635f149043c82e9045eb7a9ba45ad27 Bug 1053986 - Rename nsIFrame::GetLayoutManager to GetXULLayoutManager. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/5c37c36f2720bf76df1e088ec228e03a5ecfbe2a Bug 1053986 - Rename nsIFrame::GetClientRect to GetXULClientRect. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/3158d7e09cc3ba15d05beee805b9fe15fcf6bbf6 Bug 1053986 - Rename nsIFrame::GetVAlign to GetXULVAlign. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/c9a240b3f8a7d2fed5fbf8f2a9838cf29cf4d23b Bug 1053986 - Rename nsIFrame::GetHAlign to GetXULHAlign. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/a626ff70c178b0627d9693301b5438bd5e81dda5 Bug 1053986 - Rename nsIFrame::IsHorizontal to IsXULHorizontal, and related methods. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/39492d973854c8af2c1f3242a3fa019dba8d4ca9 Bug 1053986 - Rename nsIFrame::IsNormalDirection to IsXULNormalDirection. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/573df08134b12df73c2f390d419f2842ab40b3ba Bug 1053986 - Rename nsIFrame::Redraw to XULRedraw. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/d38adfb2e4e1dbbb91d36bb6d5f5b4b65cb76ac9 Bug 1053986 - Rename nsIFrame::RelayoutChildAtOrdinal to XULRelayoutChildAtOrdinal. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/d137bd89989d30dae2d5b9c724e4adf199100d92 Bug 1053986 - Rename nsIFrame::SetDebug to SetXULDebug. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/458314bef5313b9c8a3ddd54ecb0e92a1df8090b Bug 1053986 - Rename nsIFrame::GetDebug to GetXULDebug. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/18c217fdcd725bd91caea9f4cbe41060bbe583e8 Bug 1053986 - Rename nsIFrame::DumpBox to XULDumpBox. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/eab7fdd264f71d4618f37cccd65f9d2cb86be742 Bug 1053986 - Rename nsIFrame::AddCSSPrefSize, AddCSSMinSize, AddCSSMaxSize, and AddCSSFlex by replacing CSS with XUL. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/dae8004cf486c2262fd65920e0c886b931244b2e Bug 1053986 - Fix ordering of methods in nsIFrame.h r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/1ffc3f31dd98b47a16b16e6589512054c767ddba Bug 1053986 - Rename nsBox::BeginLayout to BeginXULLayout. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/acf9fa960f85b38665375e7f447d40cd14d19923 Bug 1053986 - Rename nsBox::EndLayout to EndXULLayout. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/583cdf2d877c00526826108c2a18eb4079200280 Bug 1053986 - Rename nsBox::DoLayout to DoXULLayout. r=dholbert
Comment 75•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f81e6e5226fb https://hg.mozilla.org/mozilla-central/rev/d4317b74afed https://hg.mozilla.org/mozilla-central/rev/8024a1e1ec3f https://hg.mozilla.org/mozilla-central/rev/0e8929a206ca https://hg.mozilla.org/mozilla-central/rev/e84de27f354c https://hg.mozilla.org/mozilla-central/rev/d57a58420319 https://hg.mozilla.org/mozilla-central/rev/4bddc023dac9 https://hg.mozilla.org/mozilla-central/rev/ec597dfc7111 https://hg.mozilla.org/mozilla-central/rev/f054013fa445 https://hg.mozilla.org/mozilla-central/rev/2942f0615e19 https://hg.mozilla.org/mozilla-central/rev/932eaf157f7f https://hg.mozilla.org/mozilla-central/rev/0f17a800a22e https://hg.mozilla.org/mozilla-central/rev/cbba01cf1942 https://hg.mozilla.org/mozilla-central/rev/3668e6c38db1 https://hg.mozilla.org/mozilla-central/rev/42a916b000f1 https://hg.mozilla.org/mozilla-central/rev/13d0946a3e53 https://hg.mozilla.org/mozilla-central/rev/fa4af7a37ad1 https://hg.mozilla.org/mozilla-central/rev/98b9e9971185 https://hg.mozilla.org/mozilla-central/rev/a92803ca0499 https://hg.mozilla.org/mozilla-central/rev/bd142212e00e https://hg.mozilla.org/mozilla-central/rev/e81b89fbb112 https://hg.mozilla.org/mozilla-central/rev/9e1500799635 https://hg.mozilla.org/mozilla-central/rev/5c37c36f2720 https://hg.mozilla.org/mozilla-central/rev/3158d7e09cc3 https://hg.mozilla.org/mozilla-central/rev/c9a240b3f8a7 https://hg.mozilla.org/mozilla-central/rev/a626ff70c178 https://hg.mozilla.org/mozilla-central/rev/39492d973854 https://hg.mozilla.org/mozilla-central/rev/573df08134b1 https://hg.mozilla.org/mozilla-central/rev/d38adfb2e4e1 https://hg.mozilla.org/mozilla-central/rev/d137bd89989d https://hg.mozilla.org/mozilla-central/rev/458314bef531 https://hg.mozilla.org/mozilla-central/rev/18c217fdcd72 https://hg.mozilla.org/mozilla-central/rev/eab7fdd264f7 https://hg.mozilla.org/mozilla-central/rev/dae8004cf486 https://hg.mozilla.org/mozilla-central/rev/1ffc3f31dd98 https://hg.mozilla.org/mozilla-central/rev/acf9fa960f85 https://hg.mozilla.org/mozilla-central/rev/583cdf2d877c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•