Bug 1196437 - Moving a sponsored tile in newtab breaks various things [r=marcosc] a=sylvestre
authorMaxim Zhilyaev <mzhilyaev@mozilla.com>
Fri, 02 Oct 2015 13:14:12 -0700
changeset 296218 6bafad1876b534df26f48494bfe9fb1084499a57
parent 296217 4f2a1db61cfdfe46666a6464faa38452b341505c
child 296219 19a60a8e02c5d6e5284b5dac528994e40b8282d5
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarcosc, sylvestre
bugs1196437
milestone43.0a2
Bug 1196437 - Moving a sponsored tile in newtab breaks various things [r=marcosc] a=sylvestre
browser/base/content/newtab/sites.js
toolkit/modules/NewTabUtils.jsm
--- a/browser/base/content/newtab/sites.js
+++ b/browser/base/content/newtab/sites.js
@@ -45,23 +45,29 @@ Site.prototype = {
   get cell() {
     let parentNode = this.node.parentNode;
     return parentNode && parentNode._newtabCell;
   },
 
   /**
    * Pins the site on its current or a given index.
    * @param aIndex The pinned index (optional).
+   * @return true if link changed type after pin
    */
   pin: function Site_pin(aIndex) {
     if (typeof aIndex == "undefined")
       aIndex = this.cell.index;
 
     this._updateAttributes(true);
-    gPinnedLinks.pin(this._link, aIndex);
+    let changed = gPinnedLinks.pin(this._link, aIndex);
+    if (changed) {
+      // render site again to remove suggested/sponsored tags
+      this._render();
+    }
+    return changed;
   },
 
   /**
    * Unpins the site and calls the given callback when done.
    */
   unpin: function Site_unpin() {
     if (this.isPinned()) {
       this._updateAttributes(false);
@@ -175,16 +181,20 @@ Site.prototype = {
     this.node.setAttribute("type", this.link.type);
 
     let titleNode = this._querySelector(".newtab-title");
     titleNode.textContent = title;
     if (this.link.titleBgColor) {
       titleNode.style.backgroundColor = this.link.titleBgColor;
     }
 
+    // remove "suggested" attribute to avoid showing "suggested" tag
+    // after site was pinned or dropped
+    this.node.removeAttribute("suggested");
+
     if (this.link.targetedSite) {
       if (this.node.getAttribute("type") != "sponsored") {
         this._querySelector(".newtab-sponsored").textContent =
           newTabString("suggested.tag");
       }
 
       this.node.setAttribute("suggested", true);
       let explanation = this._getSuggestedTileExplanation();
@@ -365,17 +375,20 @@ Site.prototype = {
         this._toggleLegalText(".newtab-sponsored", ".sponsored-explain");
         action = "sponsored";
       }
       else if (pinned && target.classList.contains("newtab-control-pin")) {
         this.unpin();
         action = "unpin";
       }
       else if (!pinned && target.classList.contains("newtab-control-pin")) {
-        this.pin();
+        if (this.pin()) {
+          // suggested link has changed - update rest of the pages
+          gAllPages.update(gPage);
+        }
         action = "pin";
       }
     }
 
     // Report all link click actions
     if (action) {
       DirectoryLinksProvider.reportSitesAction(gGrid.sites, action, tileIndex);
     }
--- a/toolkit/modules/NewTabUtils.jsm
+++ b/toolkit/modules/NewTabUtils.jsm
@@ -395,29 +395,27 @@ var PinnedLinks = {
 
     return this._links;
   },
 
   /**
    * Pins a link at the given position.
    * @param aLink The link to pin.
    * @param aIndex The grid index to pin the cell at.
+   * @return true if link changes, false otherwise
    */
   pin: function PinnedLinks_pin(aLink, aIndex) {
     // Clear the link's old position, if any.
     this.unpin(aLink);
 
     // change pinned link into a history link
-    // update all pages on link change
-    let updatePages = this._makeHistoryLink(aLink);
+    let changed = this._makeHistoryLink(aLink);
     this.links[aIndex] = aLink;
     this.save();
-    if (updatePages) {
-      AllPages.update();
-    }
+    return changed;
   },
 
   /**
    * Unpins a given link.
    * @param aLink The link to unpin.
    */
   unpin: function PinnedLinks_unpin(aLink) {
     let index = this._indexOfLink(aLink);