Provide a notification when the newtab URL changes

RESOLVED FIXED in Firefox 41

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mkaply, Assigned: nhnt11)

Tracking

Trunk
Firefox 42
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox41 fixed, firefox42 fixed)

Details

Attachments

(1 attachment)

Spun off from bug 1118285

There should be a notification that add-ons can listen for to know when the new tab value is changed (the same way they could observer the pref before.

(In reply to Florian Quèze [:florian] [:flo] from comment #63)
> (In reply to Mike Kaply [:mkaply] from comment #61)
> 
> I don't really understand the use case for your proposed notification (why
> would a user intentionally install 2 different add-ons both attempting to
> change the new tab page?), but if you think a notification is needed, I
> don't see a problem with adding one (in a new bug though).

There are a number of add-ons that offer new tab as one of multiple feature that could be turned on and off.

You need to provide a way for those add-ons that change the new tab page to coexist.

In addition, if Firefox provides a way to reset the new tab page in prefs, an add-on that is changing the new tab page needs to know when that happens so it doesn't take the new tab page the next time it starts up.

If you don't provide this, you're just going to have a bunch of add-ons that always take the new tab page at startup.
Blocks: 1118285
How exactly is this going to help? Every add-on touching the newtab URL will basically listen for this notification and change it back as soon as that happened. If the other add-on is doing the same we have a funny circle. Installing multiple "newtab add-ons" didn't make sense before, why would we support this now?
No, they wouldn't change it back. You're misunderstanding the usecase here.

I have an add-on. I keep an internal preference called "tabSearch" that when set to true means "I have the new tab page." Every time my add-on starts, I check to see if my preference is set and if it is, I take the new tab page. I do this because on shutdown, I have to reset the new tab page (because the new tab page points to a URL in my add-on) and the user could theoretically disable my add-on and uninstall it and leave behind the new tab page pointing to a page in my add-on.

I also have a preference in my add-on that allows a user to turn the new tab feature on and off.

Another add-on is installed and it takes the new tab page. My add-on (because it is a good citizen) sees that the other add-on took the new tab page and turns my internal preference OFF.

So now the next time I start, I don't take the new tab page anymore.

Without this feature, every add-on that does anything with the new tab page will simply take the new tab page at startup. And there will be no coexisting at all (and the user will have no idea which add-on has the new tab page).

The goal is not to allow add-ons to take it back, it's to allow add-ons to know when another add-on has taken it away so that they won't keep taking it at startup.

Otherwise, how is an add-on to know they shouldn't take the new tab anymore? Because either the user reset the new tab page or installed a different add-on that does it?
Another idea might be to require extensions to pass their ID to this API so that any UI associated with the new tab could show which add-on owns the new tab page.

That's a handy thing that Chrome has.
Posted patch PatchSplinter Review
This makes NewTabURL.jsm send out a notification when the newtab url changes, and adds a test.
Assignee: nobody → nhnt11
Status: NEW → ASSIGNED
Attachment #8627436 - Flags: review?(florian)
Summary: Provide a notification when the new tab value is changed → Provide a notification when the newtab URL changes
Comment on attachment 8627436 [details] [diff] [review]
Patch

Review of attachment 8627436 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/modules/test/xpcshell/test_NewTabURL.js
@@ +15,3 @@
>    Assert.ok(NewTabURL.overridden, "Newtab URL should be overridden");
>    Assert.equal(NewTabURL.get(), url, "Newtab URL should be the custom URL");
> +  notificationPromise = promiseNewtabURLNotification("about:newtab");

nit: add an empty line before this second part of the test where you test .reset().
Attachment #8627436 - Flags: review?(florian) → review+
https://hg.mozilla.org/mozilla-central/rev/9d48630bf4b8
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Target Milestone: Firefox 41 → Firefox 42
Comment on attachment 8627436 [details] [diff] [review]
Patch

I would really like this to be in the same release as bug 1118285, so if this missed the merge, I think we should uplift it to aurora.

Approval Request Comment
[Feature/regressing bug #]: bug 1118285
[User impact if declined]: inability for add-on authors to detect if another add-on has already customized the new tab page.
[Describe test coverage new/current, TreeHerder]: unit tests included in the patch.
[Risks and why]: low risk, just adding a notification.
[String/UUID change made/needed]: none.
Attachment #8627436 - Flags: approval-mozilla-aurora?
Comment on attachment 8627436 [details] [diff] [review]
Patch

Approving for uplift to Aurora. This patch has been in m-c for a while so should be stable. And the testcase makes me happy, thank you! :)
Attachment #8627436 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.