Bug 1263571 - Update switch-to-tab for reader view pages too r=liuche
authorAndrzej Hunt <ahunt@mozilla.com>
Tue, 12 Apr 2016 10:58:10 -0700
changeset 332372 9ebb4247d445421ef172c2c8e66c2cd627f2bb91
parent 332371 332bbe886f1159709a787266fc26d3e3c1384a86
child 332373 ad71189f79aa732acb0ad2518d33d46387386045
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersliuche
bugs1263571
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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;