Closed Bug 1408446 Opened 7 years ago Closed 6 years ago

Add-ons that open Windows can get an empty window

Categories

(Firefox :: Tabbed Browser, defect, P3)

x86_64
Linux
defect

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- wontfix
firefox56 --- unaffected
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- wontfix
firefox61 --- wontfix
firefox62 --- wontfix

People

(Reporter: gcp, Unassigned)

References

Details

(Keywords: regression, Whiteboard: [fxperf:p3])

Attachments

(2 files, 4 obsolete files)

I am not 100% sure what is going on, but bug 1379587 breaks wxIF: https://github.com/gcp/wxif https://addons.mozilla.org/en-US/firefox/addon/wxif/ And I bisected it to this: 9:07.92 INFO: No more inbound revisions, bisection finished. 9:07.92 INFO: Last good revision: 7e5b28819e5b382bea8cbcccf0c6402fe0e90192 9:07.92 INFO: First bad revision: 0533dbf2956818762fbbe7dbcfad738fe8383d89 9:07.92 INFO: Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=7e5b28819e5b382bea8cbcccf0c6402fe0e90192&tochange=0533dbf2956818762fbbe7dbcfad738fe8383d89 It's possible that my add-on doesn't use our APIs correctly, but in any case that bugfix seems to have unintended add-on compatibility effects.
Blocks: 1379587
(In reply to Gian-Carlo Pascutto [:gcp] from comment #0) > I am not 100% sure what is going on, but bug 1379587 breaks wxIF Can you be a bit more specific about what "breaks"? After installing the add-on in a current Nightly and using its context menu item on a picture, I get https://i.imgur.com/exU8rab.png
I am the original reporter of the phenomenon. The problem happened on my office pc (Debian linux), I can double check on Monday. Tomorrow I can test again on my home pc and provide more detail (actual OS version, libs, and so on). I have checked it on my laptop (Win 10, FF 57.0b8) and have no issues.
I bisected it on Ubuntu 16.04 so the regression might be Linux only.
OS: All → Linux
Hardware: All → x86_64
(In reply to Gian-Carlo Pascutto [:gcp] from comment #3) > I bisected it on Ubuntu 16.04 so the regression might be Linux only. Ehh, sort of false alarm, maybe. On my home linux machine the plugin window is also empty. But after resizing it, even to smaller than the original size was the exif data appears. Debian, stable, intel & nvidia drivers.
We shouldn't need to resize the window in order for the content to be visible. If anything that supports that it's not an issue with the plug-ins API use, but with Firefox's paint handling. (But note that even if it were an add-on bug, this would still mean there's compatibility impact.)
I can reproduce on Linux, and indeed resizing makes the window content appear (this likely indicates we are missing the first paint event, and removing the blank attribute when resizing forces a repaint). I'll investigate.
Attached patch set dialog=no (obsolete) (deleted) — Splinter Review
I can only reproduce the bug on Linux, and if I set dialog=no at http://searchfox.org/mozilla-central/rev/0bde08bed9b34fb6a26bcb5829dc3c180db0bb72/browser/components/extensions/ext-windows.js#147 I can no longer reproduce the bug. I don't understand why setting the 'dialog' feature would eat the first MozAfterPaint event only on Linux, but when setting dialog=no I didn't notice any obvious difference on the appearance of the popup window (I tested only on Mac and Linux). Kris, do you remember what setting the dialog feature there is expected to do?
Attachment #8919298 - Flags: feedback?(kmaglione+bmo)
Assignee: nobody → florian
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [reserve-photon-performance]
Perhaps the dialog feature causes a GTK top level window instead of a GTK popup window to be used (per bug 1109868 comment 16)?
(In reply to Florian Quèze [:florian] [:flo] from comment #7) > Kris, do you remember what setting the dialog feature there is expected to > do? It's mostly a hint to the window manager/OS about how the window should be displayed, and generally causes the minimize/maximize buttons to be disabled. But for the most part, we shouldn't really treat them differently internally.
Comment on attachment 8919298 [details] [diff] [review] set dialog=no Review of attachment 8919298 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/extensions/ext-windows.js @@ +143,5 @@ > if (createData.type === null || createData.type == "normal") { > features.push("dialog=no", "all"); > } else { > // All other types create "popup"-type windows by default. > + features.push("dialog=no", "resizable", "minimizable", "centerscreen", "titlebar", "close"); May as well just move this to the base `features` array above.
Attachment #8919298 - Flags: feedback?(kmaglione+bmo) → feedback+
Attached patch Patch v2 (obsolete) (deleted) — Splinter Review
Attachment #8920723 - Flags: review?(kmaglione+bmo)
Andy, if Kris is swamped, is there anyone else available to review this in time for 57?
Flags: needinfo?(amckay)
Attachment #8920723 - Flags: review?(kmaglione+bmo) → review+
Kris has reviewed, let me know if we can do anything more to help.
Flags: needinfo?(amckay)
Pushed by florian@queze.net: https://hg.mozilla.org/integration/mozilla-inbound/rev/2311fe1447d8 Avoid the 'dialog' features to prevent blank add-on popups on Linux, r=kmag.
Attachment #8919298 - Attachment is obsolete: true
Backed out for browser-chrome failures in browser/components/extensions/test/browser/browser_ext_windows.js: https://hg.mozilla.org/integration/mozilla-inbound/rev/7e6b2c4d23966de1fc39eca586b98a6300796271 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=2311fe1447d86bffef4ae0d9e4433325ff9d9848&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=retry&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=139776432&repo=mozilla-inbound [task 2017-10-26T08:01:11.811Z] 08:01:11 INFO - TEST-PASS | browser/components/extensions/test/browser/browser_ext_windows.js | Expect first window to be always on top - Expected: true, Actual: true - [task 2017-10-26T08:01:11.813Z] 08:01:11 INFO - Console message: [JavaScript Error: "TypeError: invalid 'in' operand browser" {file: "chrome://browser/content/tabbrowser.xml" line: 2437}] [task 2017-10-26T08:01:11.814Z] 08:01:11 INFO - _insertBrowser@chrome://browser/content/tabbrowser.xml:2437:1 [task 2017-10-26T08:01:11.815Z] 08:01:11 INFO - getRelatedElement@chrome://browser/content/tabbrowser.xml:7142:11 [task 2017-10-26T08:01:11.817Z] 08:01:11 INFO - set_selectedIndex@chrome://global/content/bindings/tabbox.xml:392:31 [task 2017-10-26T08:01:11.818Z] 08:01:11 INFO - tabs_XBL_Constructor@chrome://global/content/bindings/tabbox.xml:261:13 [task 2017-10-26T08:01:11.819Z] 08:01:11 INFO - @chrome://browser/content/tabbrowser.xml:45:9 [task 2017-10-26T08:01:11.821Z] 08:01:11 INFO - _updateNewTabVisibility@chrome://browser/content/tabbrowser.xml:5817:15 [task 2017-10-26T08:01:11.822Z] 08:01:11 INFO - tabbrowser_XBL_Constructor@chrome://browser/content/tabbrowser.xml:5874:11 [task 2017-10-26T08:01:11.823Z] 08:01:11 INFO - [task 2017-10-26T08:01:11.825Z] 08:01:11 INFO - TEST-PASS | browser/components/extensions/test/browser/browser_ext_windows.js | Expect three windows - Expected: 3, Actual: 3 - [task 2017-10-26T08:01:11.826Z] 08:01:11 INFO - Buffered messages finished [task 2017-10-26T08:01:11.828Z] 08:01:11 INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_windows.js | Expect one window - Expected: 1, Actual: 0 - [task 2017-10-26T08:01:11.829Z] 08:01:11 INFO - Stack trace: [task 2017-10-26T08:01:11.830Z] 08:01:11 INFO - @moz-extension://dbd95383-cbe9-4a75-a8f2-f6f4973705d2/%7B7ac6d032-1cb3-4f86-8a72-3644db806775%7D.js:16:7 [task 2017-10-26T08:01:11.831Z] 08:01:11 INFO - async*@moz-extension://dbd95383-cbe9-4a75-a8f2-f6f4973705d2/%7B7ac6d032-1cb3-4f86-8a72-3644db806775%7D.js:1:8 [task 2017-10-26T08:01:11.833Z] 08:01:11 INFO - [task 2017-10-26T08:01:11.834Z] 08:01:11 INFO - GECKO(2653) | JavaScript error: moz-extension://dbd95383-cbe9-4a75-a8f2-f6f4973705d2/%7B7ac6d032-1cb3-4f86-8a72-3644db806775%7D.js, line 17: TypeError: wins[0] is undefined [task 2017-10-26T08:01:11.837Z] 08:01:11 INFO - Console message: [JavaScript Error: "TypeError: wins[0] is undefined" {file: "moz-extension://dbd95383-cbe9-4a75-a8f2-f6f4973705d2/%7B7ac6d032-1cb3-4f86-8a72-3644db806775%7D.js" line: 17}] [task 2017-10-26T08:01:55.649Z] 08:01:55 INFO - Not taking screenshot here: see the one that was previously logged
Flags: needinfo?(florian)
[Tracking Requested - why for this release]: regression in 57
Attached patch Patch v3 (deleted) — Splinter Review
Tree is green with this new version: https://treeherder.mozilla.org/#/jobs?repo=try&revision=95043b716d0a0bbeb3894049c12087fad2897c49 The failures were mostly due to bitrot with the changes from bug 1406379 that landed right after I did the first version of the patch here.
Attachment #8923070 - Flags: review?(kmaglione+bmo)
Attachment #8920723 - Attachment is obsolete: true
This could still make it into Thursday's 57 beta 14 build if we can verify the fix. Is this likely an issue for Win & Mac or just Linux users?
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #18) > Is this likely an issue for Win & Mac or just Linux users? I could only reproduce on Linux.
Flags: needinfo?(florian)
Andy, can you help triage for 57? Not sure if we want to push this up.
Flags: needinfo?(amckay)
It seems low risk, however the issue can only be re-produced on Linux. I don't think that's important enough to push into Firefox 57 at this late stage given the number of users we have on Linux.
Flags: needinfo?(amckay)
Nobody seems to consider this a blocker for 57 and it is taking a long time to land anyway. Setting as wontfix for 57.
Not only Linux, Windows 10 too https://issues.adblockplus.org/ticket/5817
(In reply to mix from comment #24) > Not only Linux, Windows 10 too > https://issues.adblockplus.org/ticket/5817 I'm not sure, the developer's bug was reported on Linux as well, the track ticket describes somewhat similar symptoms but the underlying cause might be something entirely different. Windows was broken around Sept 15, so if on Windows a Nightly from a few days before that works, and afterwards it does not, then it was indeed this bug. (And it should be very very urgently re-triaged)
Sorry, didn't mean to set the tracking flag without a confirmation that this did indeed regress Windows as well.
(In reply to Gian-Carlo Pascutto [:gcp] from comment #25) > Windows was broken around Sept 15, so if on Windows a Nightly from a few And this should have said "Linux Nightly was broken"...
https://issues.adblockplus.org/ticket/5817 is definitely a different bug (window isn't blank, merely not initialized). The blank window being opened is https://issues.adblockplus.org/ticket/5985 and really a Linux-only issue.
Thanks Wladimir. Removing tracking.
Hi, what's the status of this?
I do not know if you are convinced with setting "dialog=no". Information from #1428352: This bug happens when opening a moz-extension: page; loading an http: page works as expected.
Sorry, I've been meaning to get back to this. I'm OK with setting dialog=no, but I'm concerned about how that breaks the window type queries in the latest version of the patch. We need a better solution to that than changing the tests...
> I'm concerned about how that breaks the window type queries Instead of using CHROME_LOCATIONBAR to determine if the type is "popup", a dummy CHROME_POPUP flag should be used. Checking CHROME_OPENAS_DIALOG would also be good. Florian, do you know what I mean?
Flags: needinfo?(florian)
(In reply to Javier Serrano Polo from comment #36) > Checking CHROME_OPENAS_DIALOG would also be good. This is what we were doing without the patch. > Florian, do you know what I mean? Not really, no. Feel free to steal the bug if you have ideas about how to fix it.
Flags: needinfo?(florian)
Setting "dialog=no" does not fix the bug. If I add "toolbar", "location", or anything that resizes the contents, then the contents are displayed. Should I force a resize?
https://bugzilla.mozilla.org/show_bug.cgi?id=1402110 looks to be a duplicate of this. And from the linked ticket, and some basic testing that I've done, it looks like setting `extensions.webextensions.remote` to true resolves the issue (which is false by default on Linux). Would that be related?
Setting "extensions.webextensions.remote" does not fix the bug on Firefox 57. I am tracking the painting code...
I'm NOT saying that the option resolves the bug and nothing further needs to be done. Rather, from my experience, and others, enabling the option causes windows created by extensions to paint, without manually resizing. Testing info: elementaryOS (Ubunutu Based) Firefox 57.0.4 Very simple extension. Opens an internal extension page in a new window when pressing the browser action button. Clean Profile After installing the extension and pressing the browser action I get a blank window. Restarting Firefox and the behavior continues. Went to about:config and flipped `extensions.webextensions.remote` to true. Restart Firefox. Pressing the browser action I get a window that properly shows the content. --- I also performed the test on Windows (but in reverse). Although, on the nightly branch rather than Firefox 57. Windows 7 Firefox 59.0a1 (2018-01-13) Very simple extension. Opens an internal extension page in a new window when pressing the browser action button. Clean Profile After installing the extension and pressing the browser action I get a window with the proper content. Went to about:config and flipped `extensions.webextensions.remote` to false. Restart Firefox. Pressing the browser action I get a blank window. --- So, maybe my testing is for naught, and I don't understand what I'm seeing. I'm not familiar with Firefox internals but from my external testing, to me, it looks like `remote.webextensions.remote` is doing something in regard to painting windows opened by extensions.
According to the previous comment, the bug can be reproduced on Windows, thus someone more knowledgeable than me may want to take a look; I may investigate further another day. The resize workaround works, if you are happy with it in the meantime; it is basically: > window.resizeTo(width, height - 1); > window.setTimeout(() => { > window.resizeTo(width, height); > }, 0);
If I invalidate the first child layer of root in nsDisplayList::PaintRoot(), the contents will show. resizeTo() is expected to repaint, no matter if size changed, so I could invalidate that layer whenever resizeTo() is called. Could anyone tell me whether it is acceptable to follow this solution?
Attached patch Invalidate layer (obsolete) (deleted) — Splinter Review
Here it is my proposal.
Attachment #8942773 - Flags: review?(kmaglione+bmo)
Comment on attachment 8942773 [details] [diff] [review] Invalidate layer Review of attachment 8942773 [details] [diff] [review]: ----------------------------------------------------------------- The nsWindow.cpp changes will need review from a Widget peer. I'm OK with this as a temporary workaround if they are.
Comment on attachment 8942773 [details] [diff] [review] Invalidate layer Review of attachment 8942773 [details] [diff] [review]: ----------------------------------------------------------------- ::: firefox-57.0.3.orig/browser/components/extensions/ext-windows.js @@ +190,4 @@ > if (createData.titlePreface) { > win.setTitlePreface(createData.titlePreface); > } > + window.resizeTo(window.outerWidth, window.outerHeight); window.resizeBy(0, 0); may be slightly faster.
Attachment #8942773 - Flags: review?(kmaglione+bmo) → review?(karlt)
> window.resizeBy(0, 0); may be slightly faster. Performance measurements (ms): window.resizeTo(window.outerWidth, window.outerHeight); 1.715 4.760 2.735 2.005 3.340 window.resizeBy(0, 0); 5.020 6.895 5.990 6.950 6.465
Attachment #8942773 - Flags: review?(karlt)
The patch does not fix the bug in Firefox 58.
Just for the record, setting "extensions.webextensions.remote" in Firefox 58 shows the content (patch still applied), but it breaks allowScriptsToClose: > NS_ERROR_NOT_INITIALIZED: Component returned failure code: 0xc1f30001 (NS_ERROR_NOT_INITIALIZED) [nsIMessageSender.sendAsyncMessage] ext-windows.js:19
Firefox 58 loads the page later. If I resize one second after the delayed startup, contents show up.
Attached patch Invalidate layer (2) (obsolete) (deleted) — Splinter Review
This version works with Firefox 58.
Attachment #8942773 - Attachment is obsolete: true
Attachment #8946683 - Flags: review?(karlt)
Comment on attachment 8946683 [details] [diff] [review] Invalidate layer (2) This may be a useful diagnostic patch for narrowing down the cause, but not a patch for production. If this is necessary, then it needs more explanation as to why it is necessary, but I doubt a resize is the appropriate way to invalidate and I don't know why the extensions component should need to think about invalidation. Perhaps this is just triggering an extra MozAfterPaint. There are existing things that I don't see clearly documented such as what is expected to trigger the MozAfterPaint after tab-content.js is loaded, or after documentURI changes while the browser has blank="true".
Attachment #8946683 - Flags: review?(karlt) → review-
Does the issue happen in 60? You probably should be building and testing against 60 (Nightly) We are unlikely to fix this in 58 since 58 was released a few weeks ago.
Flags: needinfo?(javier--Lx3u0NnX4Eug3hq7uRufuMOjDIwrH7)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #53) > Does the issue happen in 60? No idea. > You probably should be building and testing > against 60 (Nightly) Sorry, I cannot afford that right now. They say this bug can be reproduced in Windows by setting "extensions.webextensions.remote" to false.
Flags: needinfo?(javier--Lx3u0NnX4Eug3hq7uRufuMOjDIwrH7)
I'm able to reproduce on Nightly 60 with Windows (extensions.webextensions.remote = false [NOT THE DEFAULT]). I don't know which bits are important, but here's everything from "about:buildconfig" about:buildconfig Built from https://hg.mozilla.org/mozilla-central/rev/a8e153c55eeee93a11e87d325fb20c644421036f Build platform x86_64-pc-mingw32 Build tools z:/build/build/src/vs2017_15.4.2/VC/bin/Hostx64/x64/cl.exe 19.11.25547 -TC -nologo -wd4091 -D_HAS_EXCEPTIONS=0 -W3 -Gy -Zc:inline -Gw -wd4244 -wd4267 -we4553 z:/build/build/src/vs2017_15.4.2/VC/bin/Hostx64/x64/cl.exe 19.11.25547 -utf-8 -TP -nologo -w15038 -wd5026 -wd5027 -Zc:sizedDealloc- -wd4091 -wd4577 -D_HAS_EXCEPTIONS=0 -W3 -Gy -Zc:inline -Gw -wd4251 -wd4244 -wd4267 -wd4800 -wd4595 -we4553 -D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING -GR- -Zi -O1 -Oi -Oy- Configure options MOZ_AUTOMATION=1 'MOZILLABUILD=C:\mozilla-build' --host=x86_64-pc-mingw32 --target=x86_64-pc-mingw32 --enable-update-channel=nightly MOZILLA_OFFICIAL=1 MOZ_PGO=1 WINDOWSSDKDIR=z:/build/build/src/vs2017_15.4.2/SDK MAKECAB=z:/build/build/src/makecab.exe --enable-jemalloc --enable-js-shell RUSTC=z:/build/build/src/rustc/bin/rustc CARGO=z:/build/build/src/rustc/bin/cargo --with-mozilla-api-keyfile=c:/builds/mozilla-desktop-geoloc-api.key --with-google-api-keyfile=c:/builds/gapi.data LLVM_CONFIG=z:/build/build/src/clang/bin/llvm-config --enable-rust-simd MAKE=z:/build/build/src/mozmake.EXE --enable-crashreporter --enable-verify-mar --with-branding=browser/branding/nightly
Thanks javier and sdanele3. Florian, does this help? Is this potentially something we want to fix for 59?
Flags: needinfo?(florian)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #56) > Florian, does this help? Is this potentially > something we want to fix for 59? I don't expect to fix this myself for 59. I'm considering changing the approach I took in bug 1379587 (the goal would be to eliminate the early and undesired about:blank loads instead of hiding them) in a way that would fix both this bug and bug 1422507. I don't expect my patch to do that to be something I would be comfortable uplifting.
Flags: needinfo?(florian)
Hey florian, I just moved this to the [fxperf] tag. Would you mind setting the right priority on this based on its current state?
Flags: needinfo?(florian)
Whiteboard: [reserve-photon-performance] → [fxperf]
Assignee: florian → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(florian)
Priority: P1 → P3
Whiteboard: [fxperf] → [fxperf:p3]
I still have the problem with FF59 and the nightly on Fedora 27. Every time, I open a popup with a extension page, the page stays blank but the content is loaded. Scripts are executed and hovering an input changes the mouse cursor. If I resize the window after it was opened (waiting with await), the content is shown.
I just started having this problem with webextension popups in Firefox 60 on OSX. In Firefox 59 I wasn't affected yet.
Attached patch Invalidate layer (3) (deleted) — Splinter Review
This version works with Firefox 60. It is essentially the same as previous one.
Attachment #8946683 - Attachment is obsolete: true
I did a bit of debugging of what is presumably the same problem, starting from bug 1425829 comment 15. I *think* what was going on is that when the iframe is in-process, the display list's optimizations (which build a single display list for the painting within a given process, crossing iframe boundaries) optimize away the opacity:0 subtree (including the iframe and its contents) which causes the code that runs MozAfterPaint in the iframe to not get fired, which led the blank="true" to not get removed. But I wasn't entirely sure.
I stopped seeing this bug today (or was it yesterday?), now that bug 1357487 has landed, enabling OOP WebExtensions on Linux.
Can confirm that I no longer see this bug since FF 63.
I confirm too
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
Blocks: 1541906

I'm having this bug on Firefox 67.0 on Linux: on an extension I'm developing, pop-up window randomly appears blank when opening it. This does not happen with the same extension on chromium.
The workaround proposed in this duplicate of this bug works: https://bugzilla.mozilla.org/show_bug.cgi?id=1425829#c12

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: