Bug 1263571 - Update switch-to-tab for reader view pages too r?liuche draft
authorAndrzej Hunt <ahunt@mozilla.com>
Tue, 12 Apr 2016 10:58:10 -0700
changeset 350105 cbff68d2e8b49963c7df92de6086d6098f96cbba
parent 350104 4cdad694c54e94fce1c108dadde22d3cd3fc4b60
child 518254 178d9c7faf064349d793465e2883ad24869f770d
push id15252
push userahunt@mozilla.com
push dateTue, 12 Apr 2016 22:02:58 +0000
reviewersliuche
bugs1263571
milestone48.0a1
Bug 1263571 - Update switch-to-tab for reader view pages too r?liuche MozReview-Commit-ID: 6qKDBy2P4nK
mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java
--- a/mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java
@@ -141,21 +141,34 @@ public class TwoLinePageRow extends Line
             return;
         }
         if (tab == null) {
             return;
         }
 
         // Return early if the page URL doesn't match the current tab URL,
         // or the old tab URL.
+        // data is an empty String for ADDED/CLOSED, and contains the previous/old URL during
+        // LOCATION_CHANGE (the new URL is retrieved using tab.getURL()).
+        // tabURL and data may be about:reader URLs if the current or old tab page was a reader view
+        // page, however pageUrl will always be a plain URL (i.e. we only add about:reader when opening
+        // a reader view bookmark, at all other times it's a normal bookmark with normal URL).
         final String tabUrl = tab.getURL();
-        if (!pageUrl.equals(tabUrl) && !pageUrl.equals(data)) {
+        if (!pageUrl.equals(ReaderModeUtils.stripAboutReaderUrl(tabUrl)) &&
+            !pageUrl.equals(ReaderModeUtils.stripAboutReaderUrl(data))) {
             return;
         }
 
+        // Note: we *might* need to update the display status (i.e. switch-to-tab icon/label) if
+        // a matching tab has been opened/closed/switched to a different page. updateDisplayedUrl() will
+        // determine the changes (if any) that actually need to be made. A tab change with a matching URL
+        // does not imply that any changes are needed - e.g. if a given URL is already open in one tab, and
+        // is also opened in a second tab, the switch-to-tab status doesn't change, closing 1 of 2 tabs with a URL
+        // similarly doesn't change the switch-to-tab display, etc. (However closing the last tab for
+        // a given URL does require a status change, as does opening the first tab with that URL.)
         switch (msg) {
             case ADDED:
             case CLOSED:
             case LOCATION_CHANGE:
                 updateDisplayedUrl();
                 break;
             default:
                 break;