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)

55 Branch
defect

Tracking

(firefox-esr52 unaffected, firefox55- wontfix, firefox56+ verified, firefox57+ verified)

VERIFIED FIXED
mozilla57
Tracking Status
firefox-esr52 --- unaffected
firefox55 - wontfix
firefox56 + verified
firefox57 + verified
webextensions ?

People

(Reporter: juraj.masiar, Assigned: mixedpuppy)

Details

(Keywords: regression, regressionwindow-wanted)

Attachments

(1 file)

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
Component: Untriaged → Tabbed Browser
[Tracking Requested - why for this release]:
Regressed on beta for 55.
Component: Tabbed Browser → WebExtensions: General
Product: Firefox → Toolkit
[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.
webextensions: --- → ?
Priority: -- → P1
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
Assignee: nobody → mixedpuppy
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 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 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. :-)
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)
(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)
(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)
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.
I had kmag take a quick peak here, his response "lgtm".  I'll go ahead with the current patch.
Pushed by mixedpuppy@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f91c159b3170
fix retrieval of protocol flags, r=Gijs
Track 56+/57+ as regression.
https://hg.mozilla.org/mozilla-central/rev/f91c159b3170
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
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)
Please request Beta approval on this when you get a chance.
Flags: needinfo?(mixedpuppy)
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?
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)
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
Flags: needinfo?(brindusa.tot)
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+
Flags: qe-verify+
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)
(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.
(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)
(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+
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.