Closed Bug 1601624 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: Layout, defect)

71 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla73
Tracking Status
firefox-esr68 --- unaffected
firefox71 --- wontfix
firefox72 --- verified
firefox73 --- verified

People

(Reporter: gitladen7, Assigned: emilio)

References

(Regression)

Details

(Keywords: regression, testcase)

Attachments

(3 files)

Attached file 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.

Summary: webextension popup doesn't resize → webextension popup doesn't resize (downsize)
Status: UNCONFIRMED → NEW
Component: Untriaged → Layout
Ever confirmed: true
Flags: needinfo?(emilio)
Keywords: regression, testcase
Product: Firefox → Core
Regressed by: 1583534

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

Assignee: nobody → emilio

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

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... :/

Flags: needinfo?(emilio) → needinfo?(kmaglione+bmo)

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.

Summary: webextension popup doesn't resize (downsize) → webextension popup doesn't resize back once it hits the maximum size

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
Attachment #9114010 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e7a4f4ed6b1f
Don't suppress shrink-to-fit resize reflows on unchanged size. r=tnikkel

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).

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla73
QA Whiteboard: [qa-triaged]

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

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
Flags: qe-verify+
Flags: in-testsuite?

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

Attachment #9114010 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

(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

Flags: needinfo?(kmaglione+bmo)

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

Flags: needinfo?(kmaglione+bmo)

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

Flags: needinfo?(kmaglione+bmo)

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

Flags: needinfo?(emilio)
Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d9fe6eaf1c0d
Write an automated test for this. r=tnikkel

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

Flags: qe-verify+
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.