1567723 - New Tab Page zoom would not work properly

Status: VERIFIED FIXED

(Toolkit :: General, defect)

Description

[Alice0775 White]

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?

Comment 2

[Neil Deakin]

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

Comment 3

[Ryan VanderMeulen [:RyanVM]]

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?

Comment 4

[Neil Deakin]

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

Comment 5

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 6

[Neil Deakin]

Attachment: Bug 1567723 - Back out bug 1558919 so that zoom works on new tab page

Comment 7

[Jim Jeffery not reading bug-mail 1/2/11]

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

Comment 8

[Semtex]

Yes, WFM again.

Comment 9

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

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.

Comment 10

[Mike Conley (:mconley) (:⚙️)]

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

Comment 11

[Mike Conley (: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 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.

Comment 12

[Mike Conley (:mconley) (:⚙️)]

(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 13

[Mike Conley (:mconley) (:⚙️)]

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.

Comment 14

[Mike Conley (: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.

Comment 15

[Mike Conley (:mconley) (:⚙️)]

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

Comment 16

[Ryan VanderMeulen [:RyanVM]]

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!

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5566b1724de9

Comment 18

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

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.