Closed
Bug 1056279
Opened 10 years ago
Closed 10 years ago
Turn off enhanced tiles feature for non-en-US Firefox 33 users
Categories
(Firefox :: New Tab Page, defect)
Firefox
New Tab Page
Tracking
()
People
(Reporter: Mardak, Assigned: Mardak)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•10 years ago
|
||
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.
Assignee | ||
Comment 2•10 years ago
|
||
Doesn't have code to turn off bug 1053530 yet.
Assignee | ||
Comment 3•10 years ago
|
||
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).
Assignee | ||
Updated•10 years ago
|
Summary: Add functionality to turn off enhanced tiles feature for non-en-US aurora+ users → Turn off enhanced tiles feature for non-en-US aurora+ users
Assignee | ||
Comment 4•10 years ago
|
||
This would land directly on aurora on top of all the enhanced tiles related uplift.
Assignee: nobody → edilee
Attachment #8476447 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8477112 -
Flags: review?(adw)
Assignee | ||
Updated•10 years ago
|
Points: --- → 3
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8477112 [details] [diff] [review]
v1
Prioritizing r? for m-c patches first.
Attachment #8477112 -
Flags: review?(adw)
Assignee | ||
Updated•10 years ago
|
Flags: firefox-backlog+
Summary: Turn off enhanced tiles feature for non-en-US aurora+ users → Turn off enhanced tiles feature for non-en-US Firefox 33 users
Updated•10 years ago
|
Flags: qe-verify?
Assignee | ||
Comment 6•10 years ago
|
||
This is new code landing directly for Firefox 33 uplift. I'm not sure how extensive of tests I should be writing.. ?
Attachment #8477112 -
Attachment is obsolete: true
Attachment #8477764 -
Flags: review?(adw)
Updated•10 years ago
|
status-firefox33:
--- → affected
status-firefox34:
--- → affected
Comment 7•10 years ago
|
||
Tracking because enhanced tile is a new feature.
tracking-firefox33:
--- → +
tracking-firefox34:
--- → ?
Updated•10 years ago
|
Comment 8•10 years ago
|
||
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?
Attachment #8477764 -
Flags: review?(adw) → review+
Comment 9•10 years ago
|
||
(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...
Assignee | ||
Comment 10•10 years ago
|
||
(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!
Attachment #8477764 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 11•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Iteration: --- → 34.3
Flags: qe-verify? → qe-verify+
QA Contact: cornel.ionce
Comment 12•10 years ago
|
||
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.
Status: RESOLVED → VERIFIED
Updated•10 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•