Open
Bug 1396988
Opened 7 years ago
Updated 2 years ago
[OOP] onBeforeLinkTraversal is not handled correctly for subframes
Categories
(Firefox :: Tabbed Browser, defect, P2)
Tracking
()
NEW
Tracking | Status | |
---|---|---|
firefox56 | --- | wontfix |
firefox57 | - | fix-optional |
People
(Reporter: mixedpuppy, Unassigned)
Details
While digging into bug 1392210 further, I was using a pinned tab for behavior comparison. I noted that placing google into a pinned tab, doing a search, then clicking a link result replaces the pinned tab rather than opening in a new tab.
This is due to the use of iframes. OnBeforeLinkTraversal receives isAppTab for the docshell that received the click. tabs (and extension panels) need to use the isAppTab value for the top level frame. Chatting with bz on irc, it's not clear what docshell itself should send, but it's easier (and gives a choice of behavior) to just handle this in OnBeforeLinkTraversal.
I'll address extension panels in bug 1392210. This is for the general tab handling, someone should make a determination of the correct behavior for (pinned) tabs.
Reporter | ||
Comment 1•7 years ago
|
||
Meant to put this under Firefox originally since that's where the UX issue and code fix would be.
[Tracking Requested - why for this release]: this does affect 57, and e10s in general.
status-firefox56:
--- → affected
status-firefox57:
--- → affected
tracking-firefox57:
--- → ?
Product: Toolkit → Firefox
Version: 49 Branch → 56 Branch
Updated•7 years ago
|
Component: General → Tabbed Browser
Comment 2•7 years ago
|
||
Guessing this is wontfix for 56. Who should make that determination for 57?
Flags: needinfo?(dao+bmo)
Comment 3•7 years ago
|
||
(In reply to Shane Caraveo (:mixedpuppy) from comment #0)
> While digging into bug 1392210 further, I was using a pinned tab for
> behavior comparison. I noted that placing google into a pinned tab, doing a
> search, then clicking a link result replaces the pinned tab rather than
> opening in a new tab.
>
> This is due to the use of iframes.
Google uses iframes in the search results page? That would be surprising. My understanding is that search result links actually go to the same google domain and redirect to the destination from there, so onBeforeLinkTraversal can't do anything about this.
(In reply to Julien Cristau [:jcristau] from comment #2)
> Guessing this is wontfix for 56. Who should make that determination for 57?
Maybe I'm missing some context, but it sounds like this isn't a recent regression and as such doesn't need to be a priority for 57.
Flags: needinfo?(dao+bmo) → needinfo?(mixedpuppy)
Priority: -- → P2
Updated•7 years ago
|
Reporter | ||
Comment 4•7 years ago
|
||
Yeah, google is doing something different when in a tab, and I'm not certain it is solvable. But this particular issue is more generic.
Given a url to a page that loads a same-origin page in an iframe:
a.com/page.html
<iframe src="/framed.html">
a.com/framed.html
<a href="http://b.com/linked.html">
The onBeforeLinkTraversal code ends up using isAppTab for the iframe rather than the top level docshell. So rather than popping the linked page out into a tab, it replaces content in the iframe.
For webextensions it was appropriate to have that pop out into a tab. It may or may not be for a regular pinned page.
The "fix" is rather simple:
browser/base/content/tab-content.js
var WebBrowserChrome = {
onBeforeLinkTraversal(originalTarget, linkURI, linkNode, isAppTab) {
- return BrowserUtils.onBeforeLinkTraversal(originalTarget, linkURI, linkNode, isAppTab);
+ return BrowserUtils.onBeforeLinkTraversal(originalTarget, linkURI, linkNode, docShell.isAppTab);
},
Flags: needinfo?(mixedpuppy)
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•