implement the pref layout.show_previous_page for e10s

RESOLVED FIXED in Firefox 53

Status

()

defect
RESOLVED FIXED
3 years ago
10 months ago

People

(Reporter: tnikkel, Assigned: tnikkel)

Tracking

Trunk
mozilla53
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

3 years ago
Turns out this is easy once I remembered where the code was.
Assignee

Comment 1

3 years ago
Posted patch patchSplinter Review
Attachment #8818735 - Flags: review?(gwright)
Comment on attachment 8818735 [details] [diff] [review]
patch

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

lgtm, nice catch. I'm not a peer for view/ though, so maybe someone else will need to review this.

::: view/nsView.cpp
@@ +38,5 @@
>    mViewManager = aViewManager;
>    mDirtyRegion = nullptr;
>    mWidgetIsTopLevel = false;
> +
> +  static bool addedShowPreviousPage = false;

nit: I think the convention is normally sShowPreviousPageInitialized?
Attachment #8818735 - Flags: review?(gwright) → review+
Assignee

Comment 3

3 years ago
(In reply to George Wright (:gw280) (:gwright) from comment #2)
> nit: I think the convention is normally sShowPreviousPageInitialized?

Changed it.
Assignee

Updated

3 years ago
Attachment #8818735 - Flags: review?(mats)

Comment 4

3 years ago
Comment on attachment 8818735 [details] [diff] [review]
patch

>-  return mFrame ? mFrame->PresContext()->PresShell()->IsPaintingSuppressed() : false;
>+  return sShowPreviousPage && mFrame ? mFrame->PresContext()->PresShell()->IsPaintingSuppressed() : false;

While we're here we might as well simplify this expression.
"a ? b : false" is equivalent to "a && b", so the above
can be written as:
return sShowPreviousPage && mFrame &&
       mFrame->PresContext()->PresShell()->IsPaintingSuppressed();

Also, I can't seem to find any documentation for what the pref
"layout.show_previous_page" is supposed to do.  Can you document
the exact semantics either here or in nsSubDocumentFrame.cpp, please?
Attachment #8818735 - Flags: review?(mats) → review+
Assignee

Comment 5

3 years ago
Done.

Comment 6

3 years ago
Pushed by tnikkel@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf9d3767e4e8
Implement the pref layout.show_previous_page for e10s. r=gw280 r=mats

Comment 7

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/cf9d3767e4e8
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53

Updated

10 months ago
Product: Core → Core Graveyard
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.