Closed Bug 1521596 Opened 6 years ago Closed 5 years ago

protocol handler incognito fixups

Categories

(WebExtensions :: General, enhancement, P1)

Product:
WebExtensions
Component:
General
Type:
enhancement

Tracking

(firefox67 verified)

Status:
VERIFIED FIXED
Milestone:
mozilla67
Tracking Flags:
Tracking Status
firefox67 --- verified

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(7 files)

protocol handler chooser
(deleted), image/png
meridel
: feedback+
Details
Bug 1521596 code cleanup and eslint
(deleted), text/x-phabricator-request
Details
Bug 1521596 add support for richlistitem.disabled
(deleted), text/x-phabricator-request
Details
Bug 1521596 incognito support for protocol handlers
(deleted), text/x-phabricator-request
Details
Bug 1521596 expose private browsing flag in nsIRemoteWindowContext
(deleted), text/x-phabricator-request
Details
test-protocol.xpi
(deleted), application/x-xpinstall
Details
Bug1521596.gif
(deleted), image/gif
Details

protocol handlers wont work in private windows, however the ux needs work. Since it's not possible to load, marking at a lower priority.

There are two directions we can go.

1: let the extension message it

Currently a protocol handler page from an extension loads but without any access to any extension APIs. We could leave it at that, the extension could then recognize that APIs are not available, and show the user an error page with direction to enable the extension in private browsing.

2: handle it in external protocol service

We should check window access in nsExternalHelperAppService::LoadURI[1] and avoid loading. This would result in the app helper dialog appearing. The app helper dialog should only list helpers that can run in private windows.

[1] https://searchfox.org/mozilla-central/rev/330daedbeac2bba296d663668e0e0cf248bc6823/uriloader/exthandler/nsExternalHelperAppService.cpp#914

Thinking about this more, the whole protocol handling system would need to adjust for this. So I'd prefer the options of:

1: same as above, the page loads without API access (currently already does)
2: we do not enable the protocol_handler unless the extension has private browsing permission

mconca for input.

Flags: needinfo?(mconca)

(In reply to Shane Caraveo (:mixedpuppy) from comment #1)

Thinking about this more, the whole protocol handling system would need to adjust for this. So I'd prefer the options of:

1: same as above, the page loads without API access (currently already does)
2: we do not enable the protocol_handler unless the extension has private browsing permission

mconca for input.

This sounds good to me. We should document this behavior on MDN under the protocol_handler section.

Flags: needinfo?(mconca)
Keywords: dev-doc-needed

I'm not sure which one you're saying is ok. #1 or #2?

Flags: needinfo?(mconca)

(In reply to Shane Caraveo (:mixedpuppy) from comment #3)

I'm not sure which one you're saying is ok. #1 or #2?

...so tempted to reply with "yes"

Sorry. I think #2 follows the model for every other extension and makes the most sense.

Flags: needinfo?(mconca)

(In reply to Mike Conca [:mconca] from comment #4)

(In reply to Shane Caraveo (:mixedpuppy) from comment #3)

Sorry. I think #2 follows the model for every other extension and makes the most sense.

The general model is that it works in non-private, but not in private without permission. #2 is suggesting we do not enable it anywhere unless it has private permission.

Flags: needinfo?(mconca)

Shane and I talked this morning and it seems like we might be able to at least show the chooser dialog in PB windows, which would be a better user experience.

Flags: needinfo?(mconca)
Attached image protocol handler chooser (deleted) —

This is for text feedback. The label "Disabled in Private Browsing" will be shown when an extension handler is disallowed.

Assignee: nobody → mixedpuppy
Attachment #9042286 - Flags: feedback?(mwalkington)
Attached file Bug 1521596 code cleanup and eslint (deleted) —

Depends on D19080

Depends on D19081

Attachment #9042313 - Attachment description: Bug 1521596 fix richlistitem.disabled → Bug 1521596 add support for richlistitem.disabled
Comment on attachment 9042286 [details]
protocol handler chooser

I'm not too familiar with protocol handlers, unfortunately. Are we (Mozilla) doing the disabling for the user? I believe so. 

In that case, the verb "Disabled" works. I would adjust the string to be: "Disabled in Private Windows" (replacing "Private Browsing" with "Private Windows") to be consistent with other strings.
Flags: needinfo?(mixedpuppy)
Comment on attachment 9042286 [details]
protocol handler chooser

I'm not too familiar with protocol handlers, unfortunately. Are we (Mozilla) doing the disabling for the user? I believe so. 

In that case, the verb "Disabled" works. I would adjust the string to be: "Disabled in Private Windows" (replacing "Private Browsing" with "Private Windows") to be consistent with other UI strings.
Flags: needinfo?(mixedpuppy)
Comment on attachment 9042286 [details]
protocol handler chooser

I'm not too familiar with protocol handlers, unfortunately. Are we (Mozilla) doing the disabling for the user? I believe so. 

In that case, the verb "Disabled" works. I would adjust the string to be: "Disabled in Private Windows" (replacing "Private Browsing" with "Private Windows") to be consistent with other UI strings.
Attachment #9042286 - Flags: feedback?(mwalkington) → feedback+
Priority: P3 → P1
Pushed by scaraveo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/575458230080
expose private browsing flag in nsIRemoteWindowContext r=kmag
https://hg.mozilla.org/integration/autoland/rev/91f7275e170c
code cleanup and eslint r=rpl
https://hg.mozilla.org/integration/autoland/rev/6d7343355004
add support for richlistitem.disabled r=bgrins,aswan
https://hg.mozilla.org/integration/autoland/rev/47bfb7727024
incognito support for protocol handlers r=flod,kmag
Attached file test-protocol.xpi (deleted) —

small extension for manual testing.

  • install extension
  • open tab to mailto:foobar (ok)
  • open tab to ext+test:foobar (ok)
  • open private window
  • open tab to mailto:foobar (fail)
  • open tab to ext+test:foobar (fail)

create an html page with links to the above urls

  • in non-private window open html page
  • click on links, both should open
  • in private window open html page
  • click on links, neither should open

Further note: in cases where the url fails to open, you should get a dialog to choose an app to handle the link.

Attached image Bug1521596.gif (deleted) —

This issue is verified as fixed on Firefox 67.0a1 (20190306161300) under Win 7 64-bit and Mac OS X 10.14.1.

Please see the attached video.

Status: RESOLVED → VERIFIED
status-firefox67: fixed → verified
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: