1601624 - webextension popup doesn't resize back once it hits the maximum size

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[gitladen7]

Attachment: ext.zip

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

Load a webextension that changes the height of a element inside a webextension popup (I have attached one to this report).

This bug only occurs after updating to Firefox 71:
On Firefox 70 (popup resizes correctly): https://s.put.re/mmejCCTN.mp4
On Firefox 71 (popup fails to downsize): https://s.put.re/YQSVumCq.mp4

Actual results:

The popup height increases but it doesn't decrease.

Expected results:

The popup height should have increase and decrease.

Comment 1

[Gingerbread Man]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:73.0) Gecko/20100101 Firefox/73.0
20191205094649

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=81c77136eb578e19f946df1660e2ab2d3010df02&tochange=a7180808eaaeb815467d7794477f69a71b62a93b

Comment 2

[Emilio Cobos Álvarez (:emilio)]

I'll take a look, thanks for filing :)

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1601624 - Don't suppress shrink-to-fit resize reflows on unchanged size. r=tnikkel,botond

As the width and height arguments to ResizeReflow are treated as constraints,
not the final size, in that case.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

The patch above fixes it... Kris, do you know if we have automated tests for web-extension popup sizing? I couldn't find any from a quick skim, and I didn't find any in bug 1393116 either... :/

Comment 5

[Emilio Cobos Álvarez (:emilio)]

This is only a problem when the popup is sized past the max size that we set, that's why it's taken a bit to find.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9114010 [details]
Bug 1601624 - Don't suppress shrink-to-fit resize reflows on unchanged size. r=tnikkel,botond

Beta/Release Uplift Approval Request

• User impact if declined: Some popups from extensions may not be sized correctly.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple condition move that incorrectly suppresses some reflows that are necessary to find the final size.
• String changes made/needed: none

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e7a4f4ed6b1f
Don't suppress shrink-to-fit resize reflows on unchanged size. r=tnikkel

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Oh, and thanks for the test-case gitladen! Made the issue very clear, and it was very easy to fix (once I got to it of course).

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/e7a4f4ed6b1f

Comment 10

[Alin Ilea, Desktop QA]

Reproduced the initial issue using an old Nightly build: 20191205215330.
Verified - fixed on latest Nightly 73.0a1 (2019-12-06) (Build id: 20191206044357) on Mac OS 10.14, Windows 10 and Ubuntu 18.04

Comment 11

[Julien Cristau [:jcristau]]

Comment on attachment 9114010 [details]
Bug 1601624 - Don't suppress shrink-to-fit resize reflows on unchanged size. r=tnikkel,botond

recent regression, small fix, approved for 72.0b4

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/8919727c131e

Comment 13

[Kris Maglione [:kmag]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

The patch above fixes it... Kris, do you know if we have automated tests for web-extension popup sizing? I couldn't find any from a quick skim, and I didn't find any in bug 1393116 either... :/

Fairly extensive tests, yes:

https://searchfox.org/mozilla-central/source/browser/components/extensions/test/browser/browser_ext_pageAction_popup_resize.js
https://searchfox.org/mozilla-central/source/browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize.js

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Huh, I wonder how I missed those... I'll land a test for this based on those tests, thanks Kris.

Comment 15

[Kris Maglione [:kmag]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #14)

Huh, I wonder how I missed those... I'll land a test for this based on those tests, thanks Kris.

Not sure what info is being requested of me?

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Err, was intended to be a self-ni? to not forget to write the tests, sorry.

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1601624 - Write an automated test for this. r=tnikkel,kmag

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d9fe6eaf1c0d
Write an automated test for this. r=tnikkel

Comment 19

[Alin Ilea, Desktop QA]

Verified - fixed on latest Beta 72.0b4 (Build id: 20191206183317) on Mac OS 10.14, Windows 10 and Ubuntu 18.04

Comment 20

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/d9fe6eaf1c0d