2 browser windows are opened when click local html file. one is white/black window without UI, the other one seems normal

VERIFIED FIXED in Firefox 61

Status

()

defect
VERIFIED FIXED
a year ago
a year ago

People

(Reporter: alice0775, Assigned: florian)

Tracking

({regression})

61 Branch
Firefox 61
x86_64
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox59 unaffected, firefox60 unaffected, firefox61 verified)

Details

(Whiteboard: [fxperf:p1])

Attachments

(2 attachments, 2 obsolete attachments)

Reporter

Description

a year ago
Reproducible : always

Steps To Reproduce:
1. Nightly install as a default browser.
2. Close all browser window
3. Double click on a local html file

Actual Results:
2 browser windows(A black window and a normal window) are opened.
The black window is completely black, no UI displayed.
And the black window cannot close. you must kill from right click taskbutton.

If enabled titlebar, the black window has only titlebar.
Reporter

Comment 1

a year ago
Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e21a2a57d05dfe3c5ff8ba71131127fa781ffdd0&tochange=f026ead5dbfce9d6530429ac568ecc5544cc9b3b

Regressed by: f026ead5dbfc	Florian Quèze — Bug 1447719 - Set browser.startup.blankWindow to true on Windows and Linux, r=mconley.


And I confirmed that setting browser.startup.blankWindow = false fixed the problem.


@:florian,
Your patch seems to cause the regression. Can you look into this?
Blocks: 1447719
Flags: needinfo?(florian)
Reporter

Comment 2

a year ago
In addition to the comment #0.
If HWA is enabled, white window is opened instead of black.
Reporter

Updated

a year ago
Summary: 2 browser windows are opened when click local html file. → 2 browser windows are opened when click local html file. one is white/black window without UI, the other one seems normal
Debian Testing (KDE, Radeon RX480, WebRender)
Opening Nightly by clicking on a link in Thunderbird has the same result. I didn't run mozregression, but it's unreproducible when browser.startup.blankWindow is false.
OS: Windows 10 → All
Reporter

Comment 4

a year ago
Screenshot Nightly running on VM(sorry it so slow)
Assignee

Comment 5

a year ago
Posted patch Patch (obsolete) — Splinter Review
This is easy to fix for this specific case, but I would like to audit all the other openWindow calls in this file before requesting review.
Assignee

Updated

a year ago
Assignee: nobody → florian
Status: NEW → ASSIGNED
Assignee

Updated

a year ago
Flags: needinfo?(florian)
Whiteboard: [fxperf:p1]
Reporter

Updated

a year ago
Duplicate of this bug: 1448530
Reporter

Updated

a year ago
Duplicate of this bug: 1448561

Updated

a year ago
Duplicate of this bug: 1448678
Assignee

Comment 9

a year ago
Posted patch Patch v2 (obsolete) — Splinter Review
This turned out to require a pretty significant refactoring of the nsBrowserContentHandler.js file, but it's nicely removing code duplication :-).
Attachment #8963271 - Flags: review?(mconley)
Assignee

Updated

a year ago
Attachment #8961699 - Attachment is obsolete: true
Assignee

Comment 10

a year ago
Posted patch Patch v3Splinter Review
Fixed eslint errors.

There's still a blank window that flickers for -chrome (for the non-preference cases, which I think nobody uses), -silent (I'm not sure it's still used by anybody, it was used to set the browser as the default browser 10+ years ago), -screenshot (this sets an environment variable early during startup that I may be able to access to avoid opening the blank window).

-private doesn't work with browser.startup.blankWindow, I think that'll be for a follow-up.
Attachment #8963308 - Flags: review?(mconley)
Assignee

Updated

a year ago
Attachment #8963271 - Attachment is obsolete: true
Attachment #8963271 - Flags: review?(mconley)
The STR for this bug mentions opening up a local HTML file.  I want to add that the problem also happens opening up a link in some programs.  The program CPU-Z is just such a program.
Assignee

Comment 12

a year ago
(In reply to Florian Quèze [:florian] from comment #10)

> There's still a blank window that flickers for -chrome (for the
> non-preference cases, which I think nobody uses), -silent (I'm not sure it's
> still used by anybody, it was used to set the browser as the default browser
> 10+ years ago)

I sent https://mail.mozilla.org/pipermail/firefox-dev/2018-March/006310.html to propose removing these 2 parameters.

> -screenshot (this sets an environment variable early during
> startup that I may be able to access to avoid opening the blank window).

Actually, the blank window never gets shown when I use --screenshot on the command line. I suspect triggering the headless mode means we don't show any window, even if there's an openWindow call. Anyway, nice to take it off the list of fixes needed :-).

> -private doesn't work with browser.startup.blankWindow, I think that'll be
> for a follow-up.

I filed bug 1449925.
Comment on attachment 8963308 [details] [diff] [review]
Patch v3

Review of attachment 8963308 [details] [diff] [review]:
-----------------------------------------------------------------

That's a very nice refactor, florian! Thanks!

::: browser/components/nsBrowserContentHandler.js
@@ +172,5 @@
>  
>    return update.getProperty("openURL") || defaultOverridePage;
>  }
>  
> +function openBrowserWindow(cmdLine, urlOrUrlList, postData, forcePrivate) {

This function could use some documentation - specifically which arguments are optional, and the variations they can take (like urlOrUrlList), and the conditional arguments (postData).

You could also do postData = null and avoid the postData || null down below, though I'm not certain that's much clearer. Just neat that we can do it. :)

@@ +173,5 @@
>    return update.getProperty("openURL") || defaultOverridePage;
>  }
>  
> +function openBrowserWindow(cmdLine, urlOrUrlList, postData, forcePrivate) {
> +  let chromeURL = Services.prefs.getCharPref("browser.chromeURL");

Seems ripe for a lazy one-time getter - this is probably _never_ going to change, so it seems kinda silly to ask prefs for it each time.

(I admit, this isn't probably the hottest codepath, but still)
Attachment #8963308 - Flags: review?(mconley) → review+
Assignee

Comment 14

a year ago
(In reply to Mike Conley (:mconley) from comment #13)

Thanks for the review!

> @@ +173,5 @@
> >    return update.getProperty("openURL") || defaultOverridePage;
> >  }
> >  
> > +function openBrowserWindow(cmdLine, urlOrUrlList, postData, forcePrivate) {
> > +  let chromeURL = Services.prefs.getCharPref("browser.chromeURL");
> 
> Seems ripe for a lazy one-time getter - this is probably _never_ going to
> change, so it seems kinda silly to ask prefs for it each time.
> 
> (I admit, this isn't probably the hottest codepath, but still)

Actually there was one that I removed: https://searchfox.org/mozilla-central/rev/57bbc1ac58816dc054df242948f3ecf75e12df5f/browser/components/nsBrowserContentHandler.js#300-310
Because this is going to be called exactly once, so I don't see the point of keeping a copy of this pref value in the scope of this component.
Assignee

Comment 15

a year ago
Also, I'm tempted to entirely eliminate this "browser.chromeURL" pref, but I figured it was way out of the scope of this bug.

Comment 16

a year ago
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4c46aab1b2f
re-use the blank window whenever possible in nsBrowserContentHandler.js, r=mconley.
Assignee

Updated

a year ago
Depends on: 1450267

Comment 17

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e4c46aab1b2f
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Assignee

Updated

a year ago
Duplicate of this bug: 1450408
Reporter

Updated

a year ago
Duplicate of this bug: 1450822
Perf wins!

== Change summary for alert #12439 (as of Fri, 30 Mar 2018 09:04:06 GMT) ==

Improvements:

  6%  sessionrestore linux64 opt e10s stylo     282.75 -> 264.75
  5%  sessionrestore_no_auto_restore linux64 opt e10s stylo335.08 -> 316.92

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=12439
I have verified this bug fix on the latest nightly.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
QA Contact: ciprian.georgiu
I've also managed to reproduce this bug using an affected Nightly build (2018-03-22) and following the STR from comment 0.

This is verified fixed on Beta 61.0b10 (20180530184300) running Windows 10, 7 x64 and Ubuntu 16.04 x64.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.