Closed
Bug 1385696
Opened 7 years ago
Closed 7 years ago
extension page in tab cannot open another extension page in tab
Categories
(WebExtensions :: General, defect, P1)
Tracking
(firefox-esr52 unaffected, firefox55- wontfix, firefox56+ verified, firefox57+ verified)
VERIFIED
FIXED
mozilla57
webextensions | ? |
People
(Reporter: juraj.masiar, Assigned: mixedpuppy)
Details
(Keywords: regression, regressionwindow-wanted)
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
Gijs
:
review+
gchang
:
approval-mozilla-beta+
|
Details |
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0 Build ID: 20170727114534 Steps to reproduce: Bug introduced in 55.0b13 (64-bit), it was OK in 55.0b12. 1) install GroupSpeedDial (not related to this add-on only but can be used to reproduce): https://addons.mozilla.org/en-US/firefox/addon/groupspeeddial/ 2) open new tab 3) click any empty dial 4) click "Create new group" 5) click "Save changes" 6) middle click on that dial (it's a link to moz-extension://46e7dab8-1bbf-4f14-baf3-cd9063f5086d/dial.html#1) 7) no new tab is opened (first bug) 8) close current tab and watch as a invisible tab is now active (and can be closed with Ctr + W) (second bug) Actual results: Invisible tab was created instead of visible one using standard middle click. Expected results: New tab should be created.
[Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIIOService2.getProtocolFlags]" nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)" location: "JS frame :: chrome://browser/content/tabbrowser.xml :: addTab :: line 2568" data: no] addTab chrome://browser/content/tabbrowser.xml:2568:20 loadOneTab chrome://browser/content/tabbrowser.xml:1631:23 openLinkIn chrome://browser/content/utilityOverlay.js:449:26 contentAreaClick resource:///modules/ContentClick.jsm:97:5 receiveMessage resource:///modules/ContentClick.jsm:27:9 receiveMessage jar:file:///C:/Program%20Files/Mozilla%20Firefox/browser/omni.ja!/components/nsBrowserGlue.js:194:15 receiveMessage self-hosted:987:17
Status: UNCONFIRMED → NEW
Has STR: --- → yes
Ever confirmed: true
Comment 2•7 years ago
|
||
[Tracking Requested - why for this release]: Regressed on beta for 55.
status-firefox55:
--- → affected
status-firefox56:
--- → affected
tracking-firefox55:
--- → ?
Component: Tabbed Browser → WebExtensions: General
Keywords: regression,
regressionwindow-wanted
Product: Firefox → Toolkit
Updated•7 years ago
|
Comment 3•7 years ago
|
||
[Tracking Requested - why for this release]: Let's nominate for tracking 56, since we're probably not gonna get this in 55 this late in the cycle.
tracking-firefox56:
--- → ?
Assignee | ||
Updated•7 years ago
|
webextensions: --- → ?
Priority: -- → P1
Assignee | ||
Comment 4•7 years ago
|
||
The simple STR here is: - create addon with two pages, page1.html and page2.html (browser action button opens first) - use link in pages to navigate back and forth between them - <a href="/page2.html">another page</a> - load - on osx, you can use the context menu on the link -> "open in new tab" expected - tab opens to addon page failure: - no visible tab has opened - closing the addon tab leaves a blank space in the tab strip for me Same error as comment 1
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → mixedpuppy
Assignee | ||
Updated•7 years ago
|
Summary: Middle click won't open link to extension page, instead it creates invisible tab! → extension page in tab cannot open another extension page in tab
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8893005 [details] Bug 1385696 fix retrieval of protocol flags, https://reviewboard.mozilla.org/r/164004/#review169360
Attachment #8893005 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8893005 [details] Bug 1385696 fix retrieval of protocol flags, https://reviewboard.mozilla.org/r/164004/#review169362 ::: commit-message-44121:1 (Diff revision 1) > +Bug 1385696 fix retreival of protocol flags, r?Gijs Nit: please fix commit message spelling. :-)
Comment 8•7 years ago
|
||
Why does this break the way it does without these changes? Specifically, why does moz-extension *ever* claim to do principal inheritance?
Flags: needinfo?(mixedpuppy)
Comment 9•7 years ago
|
||
(In reply to :Gijs from comment #8) > Why does this break the way it does without these changes? Specifically, why > does moz-extension *ever* claim to do principal inheritance? Oh, so it seems the extension protocol handler doesn't implement getprotocolflags. Is there a reason not to implement it, and just give it the strictest defaults? Looks like the only variation is in whether resources are web-accessible, so we could just have the default flags claim it's not web-accessible.
Flags: needinfo?(mixedpuppy) → needinfo?(haftandilian)
Comment 10•7 years ago
|
||
(In reply to :Gijs from comment #9) > (In reply to :Gijs from comment #8) > > Why does this break the way it does without these changes? Specifically, why > > does moz-extension *ever* claim to do principal inheritance? > > Oh, so it seems the extension protocol handler doesn't implement > getprotocolflags. > > Is there a reason not to implement it, and just give it the strictest > defaults? Looks like the only variation is in whether resources are > web-accessible, so we could just have the default flags claim it's not > web-accessible. I don't know of any reasons not to do that, but would it be correct to do that when ExtensionProtocolHandler implements nsIProtocolHandlerWithDynamicFlags? I don't have much experience with protocol handlers and the flags, but I noticed the warning in SubstitutingProtocolHandler::GetProtocolFlags(): nsresult SubstitutingProtocolHandler::GetProtocolFlags(uint32_t *result) { if (mFlags.isNothing()) { NS_WARNING("Trying to get protocol flags the wrong way - use nsIProtocolHandlerWithDynamicFlags instead"); return NS_ERROR_NOT_AVAILABLE; } ... }
Flags: needinfo?(haftandilian) → needinfo?(gijskruitbosch+bugs)
Assignee | ||
Comment 11•7 years ago
|
||
In addition to the comment Haik pointed to, there is this: https://dxr.mozilla.org/mozilla-central/source/netwerk/base/nsIOService.cpp#623 nsIOService::GetProtocolFlags // We can't call DoGetProtocolFlags here because we don't have a URI. This // API is used by (and only used by) extensions, which is why it's still // around. Calling this on a scheme with dynamic flags will throw. The combination of those two comments made me choose the fix that I did, and I think it's the correct fix. We could get more input. Implementing getProtocolFlags on the extension protocol as you suggest could have other potential side affects for any web accessible urls, I'd have to study that more.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 13•7 years ago
|
||
I had kmag take a quick peak here, his response "lgtm". I'll go ahead with the current patch.
Comment 14•7 years ago
|
||
Pushed by mixedpuppy@gmail.com: https://hg.mozilla.org/integration/autoland/rev/f91c159b3170 fix retrieval of protocol flags, r=Gijs
Comment 15•7 years ago
|
||
Track 56+/57+ as regression.
status-firefox57:
--- → affected
tracking-firefox57:
--- → +
Comment 16•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f91c159b3170
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 17•7 years ago
|
||
I mean, there used to be an add-on compat argument for going with implementing getProtocolFlags as well, and warning, but I guess at this point (hi 57) it's better to just update the consumers, so this wfm.
Flags: needinfo?(gijskruitbosch+bugs)
Comment 18•7 years ago
|
||
Please request Beta approval on this when you get a chance.
status-firefox-esr52:
--- → unaffected
Flags: needinfo?(mixedpuppy)
Assignee | ||
Comment 19•7 years ago
|
||
Comment on attachment 8893005 [details] Bug 1385696 fix retrieval of protocol flags, Approval Request Comment [Feature/Bug causing the regression]: bug 1366203 [User impact if declined]: opening some extension tabs will fail [Is this code covered by automated tests?]: yes for existing use, no specific test for moz-extension schemes. [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: reproduces documented method for retrieving protocol flags [String changes made/needed]: none
Flags: needinfo?(mixedpuppy)
Attachment #8893005 -
Flags: approval-mozilla-beta?
Comment 20•7 years ago
|
||
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on the latest Nightly build?
Flags: needinfo?(brindusa.tot)
Comment 21•7 years ago
|
||
Build ID: 20170810100255 User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0 Verified as fixed on Firefox Nightly 57.0a1 on Windows 10 x 64, Mac OS X 10.12 and Ubuntu 16.04 x64.
Status: RESOLVED → VERIFIED
Updated•7 years ago
|
Flags: needinfo?(brindusa.tot)
Comment 22•7 years ago
|
||
Comment on attachment 8893005 [details] Bug 1385696 fix retrieval of protocol flags, Fix a regression and was verified. Beta56+. Should be in 56.0b3.
Attachment #8893005 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 23•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/5ee15d0b8f88
Updated•7 years ago
|
Flags: qe-verify+
Comment 24•7 years ago
|
||
I managed to reproduce this bug on an old Nightly from 2017-07-15 using Windows 10 x64. I tried to verify this bug on beta 56.0b11 using Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.11. The problem is that on beta this add-on is not working at all (the dials in a new tab page are not displayed). Any thoughts? I retested everything on the latest Nightly using the same three platforms and the bug is not reproducing there anymore. Every time you middle-click on a dial, a new tab is opened.
Flags: needinfo?(mixedpuppy)
Reporter | ||
Comment 25•7 years ago
|
||
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #24) > I managed to reproduce this bug on an old Nightly from 2017-07-15 using > Windows 10 x64. > > I tried to verify this bug on beta 56.0b11 using Windows 10 x64, Ubuntu > 16.04 x64 and Mac OS X 10.11. The problem is that on beta this add-on is not > working at all (the dials in a new tab page are not displayed). Any thoughts? Hello, I'm the author of the GroupSpeedDial add-on and we should probably discuss this through my support e-mail. This issue is probably not related with this at all - if you don't see any dials, there is a good chance your profile database was corrupted during the Firefox upgrade. There is issue created here: https://bugzilla.mozilla.org/show_bug.cgi?id=1362878 If firefox database is corrupted then my add-on won't start at all (I will introduce some message for this scenario). Re-installing add-on WON'T help, only deleting database files in your profile folder or creating new profile helps.
Assignee | ||
Comment 26•7 years ago
|
||
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #24) > I tried to verify this bug on beta 56.0b11 using Windows 10 x64, Ubuntu > 16.04 x64 and Mac OS X 10.11. The problem is that on beta this add-on is not > working at all (the dials in a new tab page are not displayed). Any thoughts? Not really, could be anything. I'd rely on my STR in comment 4 for any testing.
Flags: needinfo?(mixedpuppy)
Comment 27•7 years ago
|
||
(In reply to juraj.masiar from comment #25) > (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #24) > > I managed to reproduce this bug on an old Nightly from 2017-07-15 using > > Windows 10 x64. > > > > I tried to verify this bug on beta 56.0b11 using Windows 10 x64, Ubuntu > > 16.04 x64 and Mac OS X 10.11. The problem is that on beta this add-on is not > > working at all (the dials in a new tab page are not displayed). Any thoughts? > > Hello, I'm the author of the GroupSpeedDial add-on and we should probably > discuss this through my support e-mail. > This issue is probably not related with this at all - if you don't see any > dials, there is a good chance your profile database was corrupted during the > Firefox upgrade. There is issue created here: > https://bugzilla.mozilla.org/show_bug.cgi?id=1362878 > If firefox database is corrupted then my add-on won't start at all (I will > introduce some message for this scenario). > Re-installing add-on WON'T help, only deleting database files in your > profile folder or creating new profile helps. I've sent you an email so let's talk offline. (In reply to Shane Caraveo (:mixedpuppy) from comment #26) > (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #24) > > > I tried to verify this bug on beta 56.0b11 using Windows 10 x64, Ubuntu > > 16.04 x64 and Mac OS X 10.11. The problem is that on beta this add-on is not > > working at all (the dials in a new tab page are not displayed). Any thoughts? > > Not really, could be anything. I'd rely on my STR in comment 4 for any > testing. I didn't know how to create a new add-on, but I found another one that opens new tabs. I hope this one is good. (https://addons.mozilla.org/en-US/firefox/addon/new-tab-tools/?src=ss) Using the above add-on I managed to recreate the bug on an old version of Nightly from 2017-07-30 on Windows 10x64. I also verified if the bug is fixed on latest Nightly and beta 56.0b12 using Windows 10x64, Ubuntu 16.04 x64 and Mac OS X10.11 and it's not reproducing anymore.
Flags: qe-verify+
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•