Add nsIFrame::PresShell() for convenient access to the shell

RESOLVED FIXED in Firefox 58

Status

()

Core
Layout
P5
trivial
RESOLVED FIXED
19 days ago
15 days ago

People

(Reporter: mats, Assigned: mats)

Tracking

Trunk
mozilla58
Points:
---

Firefox Tracking Flags

(firefox58 fixed)

Details

Attachments

(4 attachments)

(Assignee)

Description

19 days ago
Frame-related code often needs the PresShell so we should
add a convenience method for it instead of writing
PresContext()->PresShell() everywhere.
(Assignee)

Comment 1

19 days ago
Created attachment 8925374 [details] [diff] [review]
part 1 - scripted changes

This is an automated conversion of PresContext()->PresShell()
to PresShell(), so it doesn't need careful review.
It changes a bit too much and doesn't compile, but I'll
fix the errors manually in a later patch.

I'll fold the first three patches into one before landing
so the commit message here covers all three.
Attachment #8925374 - Flags: review?(emilio)
(Assignee)

Comment 2

19 days ago
Created attachment 8925375 [details] [diff] [review]
part 2 - add the accessor itself
Attachment #8925375 - Flags: review?(emilio)
(Assignee)

Comment 3

19 days ago
Created attachment 8925376 [details] [diff] [review]
part 3 - manually revert a few unwanted changes from part 1
Attachment #8925376 - Flags: review?(emilio)
(Assignee)

Comment 4

19 days ago
Created attachment 8925377 [details] [diff] [review]
A few formatting improvements after mass conversion of PresContext()->PresShell() to PresShell().
Attachment #8925377 - Flags: review?(emilio)
(Assignee)

Comment 5

19 days ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=aab0ac875fca72aa11b7ba42d20d7fe63098d3de
Comment on attachment 8925375 [details] [diff] [review]
part 2 - add the accessor itself

Review of attachment 8925375 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsIFrame.h
@@ +638,5 @@
>    }
>  
>    nsPresContext* PresContext() const {
>      return StyleContext()->PresContext();
>    }

nit: Maybe leave a newline here?
Attachment #8925375 - Flags: review?(emilio) → review+
Attachment #8925374 - Flags: review?(emilio) → review+
Attachment #8925376 - Flags: review?(emilio) → review+
Comment on attachment 8925377 [details] [diff] [review]
A few formatting improvements after mass conversion of PresContext()->PresShell() to PresShell().

Review of attachment 8925377 [details] [diff] [review]:
-----------------------------------------------------------------

Neat, thanks for doing this Mats! :)

On an unrelated note, we should probably eventually fix bug 154199... I've wondered whether to take it a couple times, but I'll finish before the imagemap thingie I think...

I always have a hard time trying to figure out the reason why something hangs off the pres shell and not pres context or vice versa, and the reason for that is probably "there's no particular reason" :)
Attachment #8925377 - Flags: review?(emilio) → review+

Comment 8

16 days ago
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f91e74cdcf4c
part 1 - Add nsIFrame::PresShell() for convenient access to the shell.  r=emilio
https://hg.mozilla.org/integration/mozilla-inbound/rev/985e9aa1f587
part 2 - A few formatting improvements after mass conversion of PresContext()->PresShell() to PresShell().  r=emilio
https://hg.mozilla.org/mozilla-central/rev/f91e74cdcf4c
https://hg.mozilla.org/mozilla-central/rev/985e9aa1f587
Status: NEW → RESOLVED
Last Resolved: 15 days ago
status-firefox58: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.