Closed Bug 1567723 Opened 1 year ago Closed 1 year ago

New Tab Page zoom would not work properly

Categories

(Toolkit :: General, defect)

69 Branch
Desktop
Windows 10
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox68 --- unaffected
firefox69 + verified
firefox70 + verified

People

(Reporter: alice0775, Assigned: mconley)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: nightly-community, regression)

Attachments

(1 file)

Reproducible: almost 100%

Steps To Reproduce:

  1. Click [+] in tab bar to open New Tab Page
  2. Zoom in
  3. Restart Browser
  4. Click [+] in tab bar to open New Tab Page
    --- observe page zoomed or not
  5. Repeat Step 4

Actual Results:
Zoom indicator in Address bar shows correct zoom level.
However, New Tab Page gets 100% zoom

Expected Results:
New Tab Page should respect zoom level

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=44932f2396663a7e44434a9c562c1089ebd4422b&tochange=a9aecb4e758970a44fb6a728a8bf92e9e4187325

Regressed by: a9aecb4e758970a44fb6a728a8bf92e9e4187325 Neil Deakin — Bug 1558919, switch page zoom to use JSWindowActor so that zooming works in OOP frames, r=mconley

Neil Deakin, Can you please look into this?

Flags: needinfo?(enndeakin)
Has Regression Range: --- → yes
Has STR: --- → yes
Component: New Tab Page → Graphics
Product: Firefox → Core
Component: Graphics → General
Product: Core → Toolkit
Flags: needinfo?(dolske)
Duplicate of this bug: 1570274

Since I can't reproduce this, I'll bet this will be fixed by 1523638.

Depends on: 1523638
Flags: needinfo?(enndeakin)

That doesn't look like something we can uplift to 69. Is there anything we can do to fix this on Beta in the mean time so we don't ship this regression?

Flags: needinfo?(enndeakin)

You can back out the patch. It is only useful for fission which is ways off.

Flags: needinfo?(enndeakin)

The patch doesn't backout cleanly at this point :(. Can you please create a patch which do that? Also, hopefully bug 1523638 lands and sticks soon so we don't have to do this every cycle when a new release merges to Beta.

Flags: needinfo?(enndeakin)

This bug seems to have fixed by bug 1523638

Tested with Today's Nightly on Win10 x64
cset: https://hg.mozilla.org/mozilla-central/rev/e8fe8b0af1a7a0c64d28b4e08a9c5509d916759f

Yes, WFM again.

This bug is fixed in 70 by bug 1523638 but we don't want to uplift 1523638.
So it sounds like going ahead with the backout in comment 6 makes sense for 69 (but not 70)
I'll follow up in email with Neil.

Status: NEW → RESOLVED
Closed: 1 year ago
Flags: needinfo?(dolske)
Resolution: --- → FIXED
Assignee: nobody → enndeakin

Neil just went on PTO, so I'll work on getting this landed on beta.

Flags: needinfo?(enndeakin)

Comment on attachment 9083952 [details]
Bug 1567723 - Back out bug 1558919 so that zoom works on new tab page

Beta/Release Uplift Approval Request

  • User impact if declined: Page zoom settings on at least about:newtab will not survive restarts.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This isn't risky because it's a backout to a previously known good state that we've shipped for a long time. We're having to create the backout patch manually because of merge conflicts, but manual inspection has it put us back to the old state that we know to work.
  • String changes made/needed: None.
Attachment #9083952 - Flags: approval-mozilla-beta?
Attachment #9083952 - Attachment description: Bug 1567723, back out Bug 1567723 so that zoom works on new tab page → Bug 1567723 - Back out bug 1558919 so that zoom works on new tab page

(I've pre-emptively done the beta approval request, but I'm also going to do some local testing right now with the beta branch with the backout applied to further reduce risk)

Comment on attachment 9083952 [details]
Bug 1567723 - Back out bug 1558919 so that zoom works on new tab page

Bah, this doesn't apply cleanly to latest beta tip. Give me a few moments to post a new patch.

Attachment #9083952 - Flags: approval-mozilla-beta?
Assignee: enndeakin → mconley

Comment on attachment 9083952 [details]
Bug 1567723 - Back out bug 1558919 so that zoom works on new tab page

Beta/Release Uplift Approval Request

  • User impact if declined: Page zoom settings on at least about:newtab opened in new tabs will not survive restarts
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This isn't risky because it's a backout to a previously known good state that we've shipped for a long time. We're having to create the backout patch manually because of merge conflicts, but manual inspection has it put us back to the old state that we know to work.
  • String changes made/needed: None.
Attachment #9083952 - Flags: approval-mozilla-beta?

(Re-requested uplift after another rebase and some manual testing)

Comment on attachment 9083952 [details]
Bug 1567723 - Back out bug 1558919 so that zoom works on new tab page

Fixes a regression in 69 by reverting to previous behavior. Approved for 69.0b15. Thanks for shepherding this, Mike!

Attachment #9083952 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

I have reproduced this issue using Firefox 70.0a1 (2019.07.20) on Win 10 x64.
I can confirm this issue is fixed, I verified using Firefox 70.0a1 and 69.0b15 on Win 10 x64, Ubuntu 18.04 x64 and macOS 10.14.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Duplicate of this bug: 1566559
You need to log in before you can comment on or make changes to this bug.