Closed
Bug 1338375
Opened 7 years ago
Closed 7 years ago
Opening in a new http(s) window broken from file URL pages
Categories
(Core :: DOM: Content Processes, defect)
Core
DOM: Content Processes
Tracking
()
VERIFIED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox52 | --- | unaffected |
firefox53 | --- | disabled |
firefox54 | --- | verified |
firefox55 | --- | verified |
People
(Reporter: billm, Assigned: bobowen)
References
Details
(Whiteboard: sbwc2, sbmc2, sblc3)
Attachments
(2 files, 3 obsolete files)
4.85 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
4.58 KB,
patch
|
bobowen
:
review+
|
Details | Diff | Splinter Review |
STR: 1. Start a fresh profile. 2. In preferences, uncheck "Open new windows in a new tab instead" (in General). 3. Open a file: URL that contains a target="_blank" link. 4. Click on the link. ER: The link opens in a new window. AR: A blank window opens with no browser chrome.
Comment 1•7 years ago
|
||
(In reply to Bill McCloskey (:billm) from comment #0) > STR: > 1. Start a fresh profile. > 2. In preferences, uncheck "Open new windows in a new tab instead" (in > General). > 3. Open a file: URL that contains a target="_blank" link. > 4. Click on the link. > > ER: The link opens in a new window. > AR: A blank window opens with no browser chrome. I imagine that this is related to https://bugzilla.mozilla.org/show_bug.cgi?id=1338241 - I might end up fixing it in my patch for that bug tomorrow.
See Also: → 1338241
Reporter | ||
Comment 2•7 years ago
|
||
Sorry, I forgot to mention that I bisected. This was regressed by bug 1147911. So I don't think bug 1338241.
Assignee | ||
Comment 3•7 years ago
|
||
Looks like I never tested this particular case. If the URL is a file:// URL it works, if it is an http(s):// URL and we're going through the new RecvCreateWindowInDifferentProcess function, looks like we don't attempt to navigate the new window, doh!
Assignee: nobody → bobowencode
Blocks: 1321430
Status: NEW → ASSIGNED
status-firefox52:
--- → unaffected
status-firefox53:
--- → disabled
status-firefox54:
--- → affected
Summary: Opening in a new window broken for file URLs → Opening in a new http(s) window broken from file URLs pages
Whiteboard: sbwc2, sbmc2, sblc3
Assignee | ||
Updated•7 years ago
|
Summary: Opening in a new http(s) window broken from file URLs pages → Opening in a new http(s) window broken from file URL pages
Assignee | ||
Comment 4•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a6d5446a8da5cd078f930ca772e6a9714b8fbe6a
Assignee | ||
Comment 5•7 years ago
|
||
Attachment #8836740 -
Flags: review?(bugs)
Assignee | ||
Comment 6•7 years ago
|
||
Attachment #8836741 -
Flags: review?(bugs)
Comment 7•7 years ago
|
||
Comment on attachment 8836740 [details] [diff] [review] Part 1: Fix window settings and navigation when creating a new window in a new process So you remove the only caller of OpenWindowWithoutParent. Please remove that. Why OpenURI takes null as opener?
Attachment #8836740 -
Flags: review?(bugs) → review-
Comment 8•7 years ago
|
||
Comment on attachment 8836741 [details] [diff] [review] Part 2: Check that new http browser window opened from file URI page works please test also <a target=_blank>
Attachment #8836741 -
Flags: review?(bugs) → review-
Assignee | ||
Comment 9•7 years ago
|
||
Thanks for the reviews. (In reply to Olli Pettay [:smaug] from comment #7) > Comment on attachment 8836740 [details] [diff] [review] > Part 1: Fix window settings and navigation when creating a new window in a > new process > > So you remove the only caller of OpenWindowWithoutParent. Please remove that. Removed. > Why OpenURI takes null as opener? Originally, I'd put this in the else when aSetOpener was false anyway. At the moment aURIToLoad is only set when aSetOpener is false, but I suppose it doesn't hurt to allow for potential future uses.
Attachment #8837122 -
Flags: review?(bugs)
Assignee | ||
Updated•7 years ago
|
Attachment #8836740 -
Attachment is obsolete: true
Assignee | ||
Comment 10•7 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #8) > Comment on attachment 8836741 [details] [diff] [review] > Part 2: Check that new http browser window opened from file URI page works > > please test also <a target=_blank> Test with link added.
Attachment #8837123 -
Flags: review?(bugs)
Assignee | ||
Updated•7 years ago
|
Attachment #8836741 -
Attachment is obsolete: true
Comment 11•7 years ago
|
||
Comment on attachment 8837122 [details] [diff] [review] Part 1: Fix window settings and navigation when creating a new window in a new process >+ if (aURIToLoad) { >+ nsCOMPtr<mozIDOMWindowProxy> openerWindow; >+ if (aSetOpener && thisTabParent) { >+ openerWindow = thisTabParent->GetParentWindowOuter(); >+ } >+ nsCOMPtr<nsIBrowserDOMWindow> browserDOMWin = >+ TabParent::GetFrom(aNewTabParent)->GetBrowserDOMWindow(); I would null check GetBrowserDOMWindow
Attachment #8837122 -
Flags: review?(bugs) → review+
Updated•7 years ago
|
Attachment #8837123 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 12•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8837122 -
Attachment is obsolete: true
Assignee | ||
Comment 13•7 years ago
|
||
Comment on attachment 8841510 [details] [diff] [review] Part 1: Fix window settings and navigation when creating a new window in a new process Thanks smaug. Carrying r+ from comment 11. I've pushed to try again as it's been a while: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cac71c70aaea9fc6f2bab2f3ec4f8eaebbb5e7bd (I've fixed that trailing space issue highlighted in the Lint build locally.) (In reply to Olli Pettay [:smaug] from comment #11) > Comment on attachment 8837122 [details] [diff] [review] > >+ nsCOMPtr<nsIBrowserDOMWindow> browserDOMWin = > >+ TabParent::GetFrom(aNewTabParent)->GetBrowserDOMWindow(); > I would null check GetBrowserDOMWindow Added null check similar to the one above, also noticed that I was shadowing browserDOMWindow, so I changed that name.
Attachment #8841510 -
Flags: review+
Comment 14•7 years ago
|
||
Pushed by bobowencode@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/a009a402bd99 Part 1: Fix window settings and navigation when creating a new window in a new process. https://hg.mozilla.org/integration/mozilla-inbound/rev/08710fe142c8 Part 2: Check that new http browser window opened from file URI page works.
Comment 15•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a009a402bd99 https://hg.mozilla.org/mozilla-central/rev/08710fe142c8
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 16•7 years ago
|
||
Reproduced the initial issue on Nightly 54.0a1 (Build ID: 20170209030214) on Windows 10 x64. Verified as fixed using the latest Nightly 55.0a1 - Build ID: 20170323030203 and Aurora 54.0a2 - Build ID: 20170323004002 on Windows 10 x64, Ubuntu 16.04 x64 and Mac Os X 10.12
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•