1322308 - Read homepage (and maybe other settings?) in a WebExtension

Status: VERIFIED FIXED

(WebExtensions :: Untriaged, defect, P2)

Description

[Sören Hentzschel]

I am the author of New Tab Override and I am trying to find equivalents for the current SDK based implementations in the WebExtension world.

One option in my add-on is to use the current homepage as new tab page. It's a nice option because the user don't have to change two settings if the user always want the homepage as new tab page.

I need a way to read the homepage of the user. I don't need to change it, read only access is all what I need.

Maybe it even makes sense to implement a general solution to read "user settings". I understand that there are reasons to no longer allow to change arbitrary settings in the WebExtension world, but I think it's a valid use case to read these settings at least so that add-on developers can respect the user's settings. The homepage of the user is one example, maybe there are other cases.

Comment 1

[Ben Basson]

Also needed here is the ability to listen on preference updates, i.e. so if the user's home page is changed, add-ons can update their behaviour to reflect that change.

Comment 2

[Ben Basson]

4 months and still no decision made on whether this should go ahead or not. Can this please get on someone's agenda to make a decision? Our add-ons are not going to rewrite themselves and Firefox without existing add-on support is creeping ever closer.

Comment 3

[Andy McKay]

Since you can only set the homepage and new tab to values defined in the manifest, neither of these use cases would work anyway.

There isn't any plans between now and Firefox 57 to change how you can set those values. If this did get approved as per comment 0, it would be a low priority bug.

Comment 4

[Ben Basson]

My understanding from bug 1234150 is that you could set the new tab to a hard-coded location in the manifest and then have that redirect to whatever you like, which was my plan assuming this bug was also attended to.

I accept your prioritisation as "low" on the assumption that Mozilla know best which APIs need replacing with the most and least urgency, but at least if a design decision were made there would be a chance for _someone_ to work on it.

Comment 5

[Shane Caraveo (:mixedpuppy)]

It seems to me that you don't really need to know the value of homepage, but that you either have (or not) control of homepage from your addon.  Would that be sufficient?

Comment 6

[Sören Hentzschel]

In New Tab Override I don't need control of the homepage. I only need to know what the value of the homepage option is and when there is a change so that I can update the new tab page to the homepage. It was a much requested feature for my add-on to be able to automatically use the value of the homepage as new tab page so that the users don't have to change two settings on two different places.

Comment 7

[Ben Basson]

(In reply to Shane Caraveo (:mixedpuppy) from comment #5)
> It seems to me that you don't really need to know the value of homepage, but
> that you either have (or not) control of homepage from your addon.  Would
> that be sufficient?

The add-on does two things:

1. It reads the homepage URL(s) and takes the first (if more than one piped).
2. It sets the new tab page to be the homepage.

I don't want to change what the user's homepage is, just the new tab behaviour. If it were possible to do this without knowing the value, that would be fine by me. The only reason I need the value is to override the new tab page.

I've always tried to avoid unnecessary UI for this add-on. At the point of adding UI, I might as well scrap it and people could use the tab add-ons that do more things and have UI already.

Comment 8

[Caitlin Neiman [:caitmuenster]]

Hi all, I am confirming that this bug is on the agenda for the May 2 WebExtensions Triage meeting. 

Soren and Ben, will you be able to join us? 

Wiki: https://wiki.mozilla.org/Add-ons/Contribute/Triage#Next_Meeting

Agenda: https://docs.google.com/document/d/1vf8AaW8tKKbMn4KhsqEYhrYqVTUGaERHQmzanEp2Cls/edit#

Comment 9

[Ben Basson]

Unfortunately I won't be able to as I'll be travelling at that time, but I will check up on the outcome a couple of hours later. Thanks for sending the agenda page.

Comment 10

[Sören Hentzschel]

Unfortunately I won't be able to attent, so let me summerize my use case.

It makes sense to *no* longer allow add-ons to change every Firefox option. But it could make sense to read the Firefox options at least. And most important: I can't see how it could be harm to read options. I am sure there are a few use cases for this feature request, even if I can't tell you a lot. Firefox has a lot of options and it's up to the add-on author if he/she see a use case for his/her add-on.

For my new tab add-on (New Tab Override) the use case is to read the homepage so that I can set the homepage (or the first home page if there is more than one home page) as new tab page. It's part of my add-on because it was a frequent request so there is a need for such an option and it's not only a theoretical request.

Of cource I also want to listen to option changes. If a user changes the homepage I want to update the new tab page.

As I said there are lot of options in Firefox, so there are lot of possible ideas. For example someone could implement another view for about:config options (yeah, the user can't use the add-on to change these options…) but there could be explanations for each option in this custom view and the settings could be grouped so that the user can see all privacy settings with explanations and the current state of all these settings. The add-on could even make recommendations based on the user's settings. It's just an idea. But this kind of add-on would be possible if add-ons are able to read the options.

Comment 11

[Andy McKay]

Following on from the meeting, this seems ok to us. It notes that shane was going to update, not sure if he had anything more to say on the subject.

Comment 12

[Shane Caraveo (:mixedpuppy)]

nope, I'm good to go.

Comment 14

[Bob Silverberg [:bsilverberg]]

As this was marked as a duplicate of bug 1376604, I thought it best to copy the intent of that bug here. It is:

BrowserSettings APIs should be added to allow an extension to read the current value of the home page and the new tab page. They should not be able to set either the home page or the new tab page - these APIs will only allow for those values to be read.

Comment 15

[Shane Caraveo (:mixedpuppy)]

I still think my comment 5 is applicable here.  We probably shouldn't expose other addon data to an addon.

Comment 16

[Bob Silverberg [:bsilverberg]]

Andy, Shane has expressed concerns with exposing these values (new tab and home page overrides) to other add-ons, which is what we would be doing with this API. What are some of the use cases for this API, and how do you respond to concerns that this information could be used to determine what other add-ons are installed?

Comment 17

[Shane Caraveo (:mixedpuppy)]

(In reply to Bob Silverberg [:bsilverberg] from comment #16)
> how do you
> respond to concerns that this information could be used to determine what
> other add-ons are installed?

or simply leak a user-selected url.

I think we need to catch better notes from the design-decision meetings.  Now that I think about it, I seem to recall that my concerns were noted but it wasn't felt it is a strong issue since this requires a permission.  e.g. addons getting all your bookmarks.

Comment 18

[Andy McKay]

You can already effectively listen to those URLs and get that information using webNavigation, webRequest or the tabs API. The concerns I've seen are:
* we've got another bug about allowing dynamic setting of the value (with a permission prompt), I think add-ons should be able to read the value before setting it
* we see multiple cases where the home page or new tab page is hijacked without the users input, this would be a chance to read that and let the user know about it
* sometimes the home page is just different and an add-on might want to suggest that a user change it

Comment 19

[Eric Jung [:ericjung]]

> or simply leak a user-selected url

The user has already installed the addon and accepted its request for permissions. What is the risk? That the homepage URL is exfiltrated? We should enforce that with AMO policy, not handicap legitimate addons like New Tab Override because of possible bad apples. If I install a bookmark manager addon, I would expect it to be able to read all of my bookmarks with bookmarks.getTree(). Exfiltrating those bookmarks is something else entirely.

Comment 20

[Bob Silverberg [:bsilverberg]]

I have implemented the APIs described in comment 14, but I didn't put them into browserSettings as that's really meant to control settings. I think it would just confuse things to put some read-only settings into there, plus this implementation doesn't need any of the SettingsStore/PreferencesManager stuff that browserSettings needs.

Instead, I added methods to get the value of the home page and new tab pages to the same namespaces that are used to set those via the manifest, so we now have browser.chrome_settings_overrides.getHomepage() and browser.chrome_url_overrides.getNewTabPage().

Comment 21

[Bob Silverberg [:bsilverberg]]

Attachment: Bug 1322308 - Allow WebExtensions to read the overriden homepage and newTab values,

This introduces browser.chrome_settings_overrides.getHomepage() which will return the value of the overridden home page
and browser.chrome_url_overrides.getNewTabPage() which will return the value of the overridden new tab page.

Review commit: https://reviewboard.mozilla.org/r/157910/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/157910/

Comment 22

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8893502 [details]
Bug 1322308 - Allow WebExtensions to read the overriden homepage and newTab values,

https://reviewboard.mozilla.org/r/157910/#review170062

::: browser/components/extensions/ext-chrome-settings-overrides.js:92
(Diff revision 1)
>      }
>    }
> +
> +  getAPI(context) {
> +    return {
> +      chrome_settings_overrides: {

None of the existing js APIs use this form.  Can this be changed to chromeSettingsOverrides?  dito for chromeUrlOverrides.  This is how other manifest/api names work.  browser_action, browserAction, etc.

Comment 23

[Shane Caraveo (:mixedpuppy)]

TBH I wouldn't mind seeing something like browser.settings.*.  That feels like a more reasonable namespace than creating two new namespaces each with a single setting (with low likelyhood to grow more).

Comment 24

[Bob Silverberg [:bsilverberg]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #23)
> TBH I wouldn't mind seeing something like browser.settings.*.  That feels
> like a more reasonable namespace than creating two new namespaces each with
> a single setting (with low likelyhood to grow more).

My reasoning for putting them there was because they're the same namespaces that are used when assigning the values via manifest keys, but yes, I agree it's not ideal. I was also trying to avoid spreading logic that deals with homepage and new tab page around multiple modules, but again, I think that's a trade off that's worth in in this case. I'll make that change and push a new commit.

Comment 25

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8893502 [details]
Bug 1322308 - Allow WebExtensions to read the overriden homepage and newTab values,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/157910/diff/1-2/

Comment 26

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8893502 [details]
Bug 1322308 - Allow WebExtensions to read the overriden homepage and newTab values,

https://reviewboard.mozilla.org/r/157910/#review170062

> None of the existing js APIs use this form.  Can this be changed to chromeSettingsOverrides?  dito for chromeUrlOverrides.  This is how other manifest/api names work.  browser_action, browserAction, etc.

I addressed this by changing this around to use browserSettings, which meant I didn't need to touch chrome-settings-overrides or url-overrides.

Comment 27

[Bob Silverberg [:bsilverberg]]

A new version of this is ready to review. I addressed the read-only aspect of the API by simply ignoring any calls to set() or clear(). I can change that to throw an error if set() or clear() is called, if you think that would be better.

Comment 28

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8893502 [details]
Bug 1322308 - Allow WebExtensions to read the overriden homepage and newTab values,

https://reviewboard.mozilla.org/r/157910/#review172852

::: browser/components/extensions/test/xpcshell/test_ext_url_overrides_newtab.js:69
(Diff revision 2)
> +  }
>  
> -  let ext2 = ExtensionTestUtils.loadExtension({
> +  async function checkNewTabPageOverride(ext, expectedValue, expectedLevelOfControl) {
> +    ext.sendMessage("checkNewTabPage");
> +    let newTabPage = await ext.awaitMessage("newTabPage");
> +    dump(`\n----- in checkNewTabPageOverride, newTabPage: ${JSON.stringify(newTabPage)} -----`);

drop the dump

Comment 29

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8893502 [details]
Bug 1322308 - Allow WebExtensions to read the overriden homepage and newTab values,

https://reviewboard.mozilla.org/r/157910/#review172852

> drop the dump

Oops.

Comment 30

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8893502 [details]
Bug 1322308 - Allow WebExtensions to read the overriden homepage and newTab values,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/157910/diff/2-3/

Comment 31

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8893502 [details]
Bug 1322308 - Allow WebExtensions to read the overriden homepage and newTab values,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/157910/diff/3-4/

Comment 32

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8893502 [details]
Bug 1322308 - Allow WebExtensions to read the overriden homepage and newTab values,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/157910/diff/4-5/

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9ea423e0e3e1
Allow WebExtensions to read the overriden homepage and newTab values, r=mixedpuppy

Comment 34

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9ea423e0e3e1

Comment 35

[Sören Hentzschel]

How can this API be used? I did read https://hg.mozilla.org/mozilla-central/rev/9ea423e0e3e1 and tried the following:

I added "chrome_settings_overrides": { } to the manifest and added "browserSettings" to the permissions. In the add-on script I wrote:

> const homepage = await browser.browserSettings.homepageOverride.get({});
> console.log(homepage);

…to see the value of the homepage. The console output was:

> Object { levelOfControl: "controllable_by_this_extension", value: null }

Firefox has a homepage but the add-on says that homepage.value is null.

Comment 36

[Bob Silverberg [:bsilverberg]]

(In reply to Sören Hentzschel from comment #35)
> How can this API be used? I did read
> https://hg.mozilla.org/mozilla-central/rev/9ea423e0e3e1 and tried the
> following:
> 
> I added "chrome_settings_overrides": { } to the manifest and added
> "browserSettings" to the permissions. In the add-on script I wrote:
> 
> > const homepage = await browser.browserSettings.homepageOverride.get({});
> > console.log(homepage);
> 
> …to see the value of the homepage. The console output was:
> 
> > Object { levelOfControl: "controllable_by_this_extension", value: null }
> 
> Firefox has a homepage but the add-on says that homepage.value is null.

This reports the value of the homepage *if overridden*. null means that no extension has overridden the home page.

Comment 37

[Sören Hentzschel]

That's bad… I opened this ticket because for New Tab Override I wanted to read the homepage. And the title of this ticket still reads: "Read homepage (and maybe other settings?) in a WebExtension". Also in comment 14 you said: "BrowserSettings APIs should be added to allow an extension to read the current value of the home page and the new tab page." - no word about that it only works if another add-on has overwritten the homepage.

I already promised my users that Mozilla will implement this feature because of the communication in this ticket. And I don't understand how this API can be useful if it only works if another add-on has overwritten the homepage, either I want do know the homepage or I don't want to know that. How make it sense that it only works for the big minority of users (because most user don't have add-ons installed which override the homepage)? :(

tl;dr: So this patch didn't implemented what this request was about. :(

Comment 38

[Ben Basson]

If all this achieves is knowing the homepage URL if overridden by an add-on, then it doesn't satisfy the requirement for New Tab Homepage (108,000 active daily users) or New Tab Override (105,000 active daily users), and it's hard to see how it could be in any way interpreted to meet the description in comment #0.

> One option in my add-on is to use the current homepage as new tab page. It's
> a nice option because the user don't have to change two settings if the user
> always want the homepage as new tab page.
> 
> I need a way to read the homepage of the user. I don't need to change it,
> read only access is all what I need.

Comment 39

[Sören Hentzschel]

:andym, since the patch did not implement the requested enhancement and I thought my original request was approved: do I have to open a new ticket and hope that there will be again an "okay" on a triaging meeting to get this implemented? :-/

Comment 40

[Bob Silverberg [:bsilverberg]]

TBH I'm not entirely sure why it was implemented this way. I can update it to return the current value of the home page / new tab page regardless of whether it's been overridden or not. I assume that will address the use cases that are areas of concern?

Comment 41

[Sören Hentzschel]

Yes. It would be awesome if you could update it to return the current value of the homepage, even if it's not overridden by an add-on. Thank you! :)

Comment 42

[Sören Hentzschel]

Thank you, Bob. It works after the landing of bug 1397081. But I have one further question: is there a reason why browserSettings can't be used as optional permission?

Error: Type error for parameter permissions (Error processing permissions.0: Value "browserSettings" must either: be one of ["clipboardRead", "clipboardWrite", "geolocation", "idle", "notifications"], must either [be one of ["<all_urls>"], match the pattern /^(https?|wss?|file|ftp|\*):\/\/(\*|\*\.[^*/]+|[^*/]+)\/.*$/, or match the pattern /^file:\/\/\/.*$/], be one of ["bookmarks"], be one of ["find"], be one of ["history"], be one of ["activeTab", "tabs"], be one of ["cookies"], be one of ["topSites"], be one of ["webNavigation"], or be one of ["webRequest", "webRequestBlocking"]) for permissions.request.

Comment 43

[Bob Silverberg [:bsilverberg]]

browserSettings originally landed as a required permission, but I think it would be okay for it to be optional. Andrew, are there any issues with changing it to be optional at this point?

Comment 44

[Andrew Swan [:aswan]]

(In reply to Bob Silverberg [:bsilverberg] from comment #43)
> browserSettings originally landed as a required permission, but I think it
> would be okay for it to be optional. Andrew, are there any issues with
> changing it to be optional at this point?

Nothing comes to mind.

Comment 45

[Bob Silverberg [:bsilverberg]]

I opened bug 1399176 to convert browserSettings to an optional permission.

Comment 46

[Will Bamberg [:wbamberg]]

I've added pages for these settings:

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/browserSettings/homepageOverride
https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/browserSettings/newTabPageOverride

Please let me now if this covers it.

(BTW, I tried writing them, and it returns undefined. Is this intentional? I might have expected it to return false, as per https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/types/BrowserSetting/set).

Comment 47

[Bob Silverberg [:bsilverberg]]

(In reply to Will Bamberg [:wbamberg] from comment #46)
> I've added pages for these settings:
> 
> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/
> browserSettings/homepageOverride
> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/
> browserSettings/newTabPageOverride
> 
> Please let me now if this covers it.
> 

Thanks Will, the docs look good.

> (BTW, I tried writing them, and it returns undefined. Is this intentional? I
> might have expected it to return false, as per
> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/types/
> BrowserSetting/set).

That's a good idea. Do you mind opening a follow-up bug for that?

Comment 48

[Will Bamberg [:wbamberg]]

Thanks for the review.

> That's a good idea. Do you mind opening a follow-up bug for that?

-> https://bugzilla.mozilla.org/show_bug.cgi?id=1408472

Comment 49

[marius.santa]

Attachment: read homepage.gif

Issue reproduced on Firefox 55.0.4 (20170826031719) under Wind 10 64-bit. 

This issue is verified as fixed on Firefox 57.0b11 (20171023180840) under Wind 10 64-bit and Mac OS X 10.13.