Backed out changeset 0c69682ddb67 (bug 1404193) for eslint failure at browser/extensions/onboarding/content/onboarding.js:1141: Unsafe assignment to innerHTML. r=backout on a CLOSED TREE
authorSebastian Hengst <archaeopteryx@coole-files.de>
Tue, 24 Oct 2017 09:08:31 +0200
changeset 685704 9c968cad8e6ae47aec9ec71d5ace795c2468a82b
parent 685703 01c28e18b63c00989775445dbeec6adc697b17b7
child 685705 a59e00e80ba49ba45aeaa4524b991395775ee285
push id86016
push userkgupta@mozilla.com
push dateWed, 25 Oct 2017 01:53:44 +0000
reviewersbackout
bugs1404193
milestone58.0a1
backs out0c69682ddb674300d182e1a4c63c88c437c789fa
Backed out changeset 0c69682ddb67 (bug 1404193) for eslint failure at browser/extensions/onboarding/content/onboarding.js:1141: Unsafe assignment to innerHTML. r=backout on a CLOSED TREE
browser/extensions/onboarding/README.md
browser/extensions/onboarding/content/onboarding.js
browser/extensions/onboarding/test/browser/browser_onboarding_skip_tour.js
browser/extensions/onboarding/test/browser/head.js
--- a/browser/extensions/onboarding/README.md
+++ b/browser/extensions/onboarding/README.md
@@ -44,44 +44,23 @@ We would apply some rules:
 * For accessibility, images that are for presentation only should have `role="presentation"` attribute.
 
 ## How to change the order of tours
 
 Edit `browser/app/profile/firefox.js` and modify `browser.onboarding.newtour` for the new user tour or `browser.onboarding.updatetour` for the update user tour. You can change the tour list and the order by concate `tourIds` with `,` sign. You can find available `tourId` from `onboardingTourset` in `onboarding.js`.
 
 ## How to pump tour set version after update tours
 
-We only update the tourset version when we have different **update** tourset. Update the new tourset **does not** require update the tourset version.
-
 The tourset version is used to track the last major tourset change version. The `tourset-version` pref store the major tourset version (ex: `1`) but not the current browser version. When browser update to the next version (ex: 58, 59) the tourset pref is still `1` if we didn't do any major tourset update.
 
 Once the tour set version is updated (ex: `2`), onboarding overlay should show the update tour to the updated user (ex: update from v56 -> v57), even when user has watched the previous tours or preferred to hide the previous tours.
 
 Edit `browser/app/profile/firefox.js` and set `browser.onboarding.tourset-version` as `[tourset version]` (in integer format).
 
 For example, if we update the tourset in v60 and decide to show all update users the tour, we set `browser.onboarding.tourset-version`  as `3`.
 
 ## Icon states
 
 Onboarding module has two states for its overlay icon: `default` and `watermark`.
 By default, it shows `default` state.
 When either tours or notifications are all completed, the icon changes to the `watermark` state.
 The icon state is stored in `browser.onboarding.state`.
 When `tourset-version` is updated, or when we detect the `tour-type` is changed to `update`, icon state will be changed back to the `default` state.
-
-## Customizable preferences
-
-Here are current support preferences that allow to customize the Onboarding's behavior.
-
-| PREF | DESCRIPTION | DEFAULT |
-|-----|-------------|:-----:|
-| `browser.onboarding.enabled` | disable onboarding experience entirely | true
-| `browser.onboarding.notification.finished` | Decide if we want to hide the notification permanently. | false
-| `browser.onboarding.notification.mute-duration-on-first-session-ms` |Notification mute duration. It also effect when the speech bubble is hidden and turned into the blue dot | 300000 (5 Min)
-| `browser.onboarding.notification.max-life-time-all-tours-ms` | Notification tours will all hide after this period | 1209600000 (10 Days)
-| `browser.onboarding.notification.max-life-time-per-tours-ms` | Per Notification tours will hide and show the next tour after this period | 432000000 (5 Days)
-| `browser.onboarding.notification.max-prompt-count-per-tour` | Each tour can only show the specific times in notification bar if user didn't interact with the tour notification. | 8
-| `browser.onboarding.newtour` | The tourset of new user tour. | performance,private,screenshots,addons,customize,default
-| `browser.onboarding.newtour.tooltip` | The string id which is shown in the new user tour's speech bubble. The preffered length is 2 lines. Should use `%S` to denote Firefox (brand short name) in string, or use `%1$S` if the name shows more than 1 time. | `onboarding.overlay-icon-tooltip2`
-| `browser.onboarding.updatetour` | The tourset of new user tour. | performance,library,screenshots,singlesearch,customize,sync
-| `browser.onboarding.updatetour.tooltip` | The string id which is shown in the update user tour's speech bubble. The preffered length is 2 lines. Should use `%S` to denote Firefox (brand short name) in string, or use `%1$S` if the name shows shows more than 1 time. | `onboarding.overlay-icon-tooltip-updated2`
-| `browser.onboarding.default-icon-src` | The default icon url. Should be svg or at least 64x64 | `chrome://branding/content/icon64.png`
-| `browser.onboarding.watermark-icon-src` | The watermark icon url. Should be svg or at least 64x64 | `resource://onboarding/img/watermark.svg`
--- a/browser/extensions/onboarding/content/onboarding.js
+++ b/browser/extensions/onboarding/content/onboarding.js
@@ -17,20 +17,19 @@ const UITOUR_JS_URI = "resource://onboar
 const TOUR_AGENT_JS_URI = "resource://onboarding/onboarding-tour-agent.js";
 const BRAND_SHORT_NAME = Services.strings
                      .createBundle("chrome://branding/locale/brand.properties")
                      .GetStringFromName("brandShortName");
 const PROMPT_COUNT_PREF = "browser.onboarding.notification.prompt-count";
 const ONBOARDING_DIALOG_ID = "onboarding-overlay-dialog";
 const ONBOARDING_MIN_WIDTH_PX = 960;
 const SPEECH_BUBBLE_MIN_WIDTH_PX = 1130;
-const SPEECH_BUBBLE_NEWTOUR_STRING_ID = "onboarding.overlay-icon-tooltip2";
-const SPEECH_BUBBLE_UPDATETOUR_STRING_ID = "onboarding.overlay-icon-tooltip-updated2";
 const ICON_STATE_WATERMARK = "watermark";
 const ICON_STATE_DEFAULT = "default";
+
 /**
  * Add any number of tours, key is the tourId, value should follow the format below
  * "tourId": { // The short tour id which could be saved in pref
  *   // The unique tour id
  *   id: "onboarding-tour-addons",
  *   // The string id of tour name which would be displayed on the navigation bar
  *   tourNameId: "onboarding.tour-addon",
  *   // The method returing strings used on tour notification
@@ -1126,82 +1125,62 @@ class Onboarding {
       event: "overlay-skip-tour",
       session_key: this._session_key
     });
   }
 
   _renderOverlay() {
     let div = this._window.document.createElement("div");
     div.id = "onboarding-overlay";
-    let skipButtonMarkup = "";
-    if (!Services.prefs.getBoolPref("browser.onboarding.skip-tour-button.hide", false)) {
-      let label = this._bundle.GetStringFromName("onboarding.skip-tour-button-label");
-      skipButtonMarkup = `<button id="onboarding-skip-tour-button" class="onboarding-action-button">${label}</button>`;
-    }
     // We use `innerHTML` for more friendly reading.
     // The security should be fine because this is not from an external input.
     div.innerHTML = `
       <div role="dialog" tabindex="-1" aria-labelledby="onboarding-header">
         <header id="onboarding-header"></header>
         <nav>
           <ul id="onboarding-tour-list" role="tablist"></ul>
         </nav>
         <footer id="onboarding-footer">
-          ${skipButtonMarkup}
+          <button id="onboarding-skip-tour-button" class="onboarding-action-button"></button>
         </footer>
         <button id="onboarding-overlay-close-btn" class="onboarding-close-btn"></button>
       </div>
     `;
 
     this._dialog = div.querySelector(`[role="dialog"]`);
     this._dialog.id = ONBOARDING_DIALOG_ID;
+
+    div.querySelector("#onboarding-skip-tour-button").textContent =
+      this._bundle.GetStringFromName("onboarding.skip-tour-button-label");
     div.querySelector("#onboarding-header").textContent =
       this._bundle.GetStringFromName("onboarding.overlay-title2");
     let closeBtn = div.querySelector("#onboarding-overlay-close-btn");
     closeBtn.setAttribute("title",
       this._bundle.GetStringFromName("onboarding.overlay-close-button-tooltip"));
     return div;
   }
 
   _renderOverlayButton() {
     let button = this._window.document.createElement("button");
-    // support customize speech bubble string via pref
-    let tooltipStringPrefId = "";
-    let defaultTourStringId = "";
-    if (this._tourType === "new") {
-      tooltipStringPrefId = "browser.onboarding.newtour.tooltip";
-      defaultTourStringId = SPEECH_BUBBLE_NEWTOUR_STRING_ID;
-    } else {
-      tooltipStringPrefId = "browser.onboarding.updatetour.tooltip";
-      defaultTourStringId = SPEECH_BUBBLE_UPDATETOUR_STRING_ID;
-    }
-    let tooltip = "";
-    try {
-      let tooltipStringId = Services.prefs.getStringPref(tooltipStringPrefId, defaultTourStringId);
-      tooltip = this._bundle.formatStringFromName(tooltipStringId, [BRAND_SHORT_NAME], 1);
-    } catch (e) {
-      Cu.reportError(`the provided ${tooltipStringPrefId} string is in wrong format `, e);
-      // fallback to defaultTourStringId to proceed
-      tooltip = this._bundle.formatStringFromName(defaultTourStringId, [BRAND_SHORT_NAME], 1);
-    }
+    let tooltipStringId = this._tourType === "new" ?
+      "onboarding.overlay-icon-tooltip2" : "onboarding.overlay-icon-tooltip-updated2";
+    let tooltip = this._bundle.formatStringFromName(tooltipStringId, [BRAND_SHORT_NAME], 1);
     button.setAttribute("aria-label", tooltip);
     button.id = "onboarding-overlay-button";
     button.setAttribute("aria-haspopup", true);
     button.setAttribute("aria-controls", `${ONBOARDING_DIALOG_ID}`);
     let defaultImg = this._window.document.createElement("img");
     defaultImg.id = "onboarding-overlay-button-icon";
     defaultImg.setAttribute("role", "presentation");
-    defaultImg.src = Services.prefs.getStringPref("browser.onboarding.default-icon-src",
-      "chrome://branding/content/icon64.png");
+    defaultImg.src = "chrome://branding/content/icon64.png";
     button.appendChild(defaultImg);
     let watermarkImg = this._window.document.createElement("img");
     watermarkImg.id = "onboarding-overlay-button-watermark-icon";
     watermarkImg.setAttribute("role", "presentation");
-    watermarkImg.src = Services.prefs.getStringPref("browser.onboarding.watermark-icon-src",
-      "resource://onboarding/img/watermark.svg");
+    watermarkImg.src = "resource://onboarding/img/watermark.svg";
     button.appendChild(watermarkImg);
     return button;
   }
 
   _loadTours(tours) {
     let itemsFrag = this._window.document.createDocumentFragment();
     let pagesFrag = this._window.document.createDocumentFragment();
     for (let tour of tours) {
--- a/browser/extensions/onboarding/test/browser/browser_onboarding_skip_tour.js
+++ b/browser/extensions/onboarding/test/browser/browser_onboarding_skip_tour.js
@@ -21,22 +21,8 @@ add_task(async function test_skip_onboar
   let overlayClosedPromise = promiseOnboardingOverlayClosed(tab.linkedBrowser);
   await BrowserTestUtils.synthesizeMouseAtCenter("#onboarding-skip-tour-button", {}, tab.linkedBrowser);
   await overlayClosedPromise;
   await Promise.all(expectedPrefUpdates);
   await assertWatermarkIconDisplayed(tab.linkedBrowser);
 
   await BrowserTestUtils.removeTab(tab);
 });
-
-add_task(async function test_hide_skip_button_via_perf() {
-  resetOnboardingDefaultState();
-  Preferences.set("browser.onboarding.skip-tour-button.hide", true);
-
-  let tab = await openTab(ABOUT_NEWTAB_URL);
-  await promiseOnboardingOverlayLoaded(tab.linkedBrowser);
-  await BrowserTestUtils.synthesizeMouseAtCenter("#onboarding-overlay-button", {}, tab.linkedBrowser);
-  await promiseOnboardingOverlayOpened(tab.linkedBrowser);
-
-  ok(!content.document.querySelector("#onboarding-skip-tour-button"), "should not render the skip button");
-
-  await BrowserTestUtils.removeTab(tab);
-});
--- a/browser/extensions/onboarding/test/browser/head.js
+++ b/browser/extensions/onboarding/test/browser/head.js
@@ -35,17 +35,16 @@ function resetOnboardingDefaultState() {
   Preferences.set("browser.onboarding.notification.finished", false);
   Preferences.set("browser.onboarding.notification.mute-duration-on-first-session-ms", 300000);
   Preferences.set("browser.onboarding.notification.max-life-time-per-tour-ms", 432000000);
   Preferences.set("browser.onboarding.notification.max-life-time-all-tours-ms", 1209600000);
   Preferences.set("browser.onboarding.notification.max-prompt-count-per-tour", 8);
   Preferences.reset("browser.onboarding.notification.last-time-of-changing-tour-sec");
   Preferences.reset("browser.onboarding.notification.prompt-count");
   Preferences.reset("browser.onboarding.notification.tour-ids-queue");
-  Preferences.reset("browser.onboarding.skip-tour-button.hide");
   TOUR_IDs.forEach(id => Preferences.reset(`browser.onboarding.tour.${id}.completed`));
   UPDATE_TOUR_IDs.forEach(id => Preferences.reset(`browser.onboarding.tour.${id}.completed`));
 }
 
 function setTourCompletedState(tourId, state) {
   Preferences.set(`browser.onboarding.tour.${tourId}.completed`, state);
 }