1056279 - Turn off enhanced tiles feature for non-en-US Firefox 33 users

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect)

Description

[Ed Lee :Mardak]

There's some new functionality that shouldn't happen for non-en-US users on aurora+.

Bug 1037091 adds customize menu with items
-> remove the menu to avoid strings and leave it as a toggle to show/hide

Bug 1053530 shows the messaging on first open
-> don't show

Bug 1042214 sends pings
-> don't send ping

Comment 1

[Ed Lee :Mardak]

Also turning off directory links to prevent showing any content that requires the sponsored indicator from bug 1040369.

And override fetching potentially remote links such as those from bug 1042876.

Comment 2

[Ed Lee :Mardak]

Attachment: wip for aurora

Doesn't have code to turn off bug 1053530 yet.

Comment 3

[Ed Lee :Mardak]

There's a bunch of "// Bug 1056293 will remove this code at the appropriate time" because originally I was thinking this would land on m-c, but we could just land this directly to aurora and it'll never need bug 1056293 to remove (this code from nightly).

Comment 4

[Ed Lee :Mardak]

Attachment: v1

This would land directly on aurora on top of all the enhanced tiles related uplift.

Comment 5

[Ed Lee :Mardak]

Comment on attachment 8477112 [details] [diff] [review]
v1

Prioritizing r? for m-c patches first.

Comment 6

[Ed Lee :Mardak]

Attachment: v1

This is new code landing directly for Firefox 33 uplift. I'm not sure how extensive of tests I should be writing.. ?

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Tracking because enhanced tile is a new feature.

Comment 8

[Drew Willcoxon :adw]

Comment on attachment 8477764 [details] [diff] [review]
v1

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

::: browser/base/content/newtab/customize.js
@@ +47,5 @@
>  
>    showPanel: function() {
> +    if (!DirectoryLinksProvider.enabled) {
> +      gAllPages.enabled = !gAllPages.enabled;
> +      return;

So for non-en-US, the old tiny grid icon is still replaced with the new gear icon, but clicking it has the old toggling behavior instead of showing the new customize panel, right?

::: toolkit/modules/DirectoryLinksProvider.jsm
@@ +84,5 @@
>    }),
>  
>    get _linksURL() {
> +    if (!this.enabled) {
> +      return "data:text/plain,{}";

Shouldn't this be application/json?

Comment 9

[Drew Willcoxon :adw]

(In reply to Ed Lee :Mardak from comment #6)
> I'm not sure how extensive of tests I should be writing.. ?

Oh, I meant to comment on this.  This patch should definitely have a test, don't you think?  As part of uplifting all of this to 33, the existing tests will come along, right?  Tests run under en-US by default, so the en-US case would already be tested.  So I think the new tests for this bug would change the locale to non-en-US, and then check:

* clicking the gear icon toggles the page instead of showing the panel
* the intro is never shown
* DirectoryLinksProvider.enabled is false
* DirectoryLinksProvider.getLinks returns an empty array
* DirectoryLinksProvider writes an empty JSON blob to disk
* DirectoryLinksProvider.reportSitesAction doesn't phone home, somehow...

Comment 10

[Ed Lee :Mardak]

Attachment: for aurora checkin

(In reply to Drew Willcoxon :adw (Away 8/27–9/2) from comment #8)
> >    showPanel: function() {
> > +    if (!DirectoryLinksProvider.enabled) {
> > +      gAllPages.enabled = !gAllPages.enabled;
> So for non-en-US, the old tiny grid icon is still replaced with the new gear
> icon, but clicking it has the old toggling behavior instead of showing the
> new customize panel, right?
Correct. Reuses the now-unremoved toggle strings too.

> >    get _linksURL() {
> > +    if (!this.enabled) {
> > +      return "data:text/plain,{}";
> Shouldn't this be application/json?
Fixed.


(In reply to Drew Willcoxon :adw (Away 8/27–9/2) from comment #9)
> As part of uplifting all of this to 33, the existing tests will come along
Yup!

> So I think the new tests for this bug would change the locale to non-en-US
> * clicking the gear icon toggles the page instead of showing the panel
> * the intro is never shown
> * DirectoryLinksProvider.enabled is false
> * DirectoryLinksProvider.getLinks returns an empty array
> * DirectoryLinksProvider writes an empty JSON blob to disk
> * DirectoryLinksProvider.reportSitesAction doesn't phone home, somehow...
Done. Done. Done. Done. Done. Done!

Comment 11

[Ed Lee :Mardak]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d9a70b065d65

Comment 12

[Cornel Ionce [:noni]]

Verified fixed on Windows 7 64bit, Mac OS X 10.8.5 and Ubuntu 14.04 32bit using latest Aurora, build ID: 20140828004002.

Tested with the ach/ar/es-ES/fr/de and ja-JP locales and the enhanced tiles is disabled.
Clicking the gear button from the newtab page will switch to blank page and back.