Bug 923086 - "Add to reading list" button does not save it's state, r=lucasr

Part 2 - Actual final bug solution, tie into Part 1
This commit is contained in:
Mark Capella 2013-10-14 09:58:16 -04:00
parent 4dd7549869
commit d869dc3636
7 changed files with 75 additions and 19 deletions

View File

@ -340,6 +340,26 @@ abstract public class BrowserApp extends GeckoApp
});
}
void handleReaderListStatusRequest(final String url) {
ThreadUtils.postToBackgroundThread(new Runnable() {
@Override
public void run() {
final int inReadingList = BrowserDB.isReadingListItem(getContentResolver(), url) ? 1 : 0;
final JSONObject json = new JSONObject();
try {
json.put("url", url);
json.put("inReadingList", inReadingList);
} catch (JSONException e) {
Log.e(LOGTAG, "JSON error - failed to return inReadingList status", e);
return;
}
GeckoAppShell.sendEventToGecko(GeckoEvent.createBroadcastEvent("Reader:ListStatusReturn", json.toString()));
}
});
}
void handleReaderAdded(int result, final String title, final String url) {
if (result != READER_ADD_SUCCESS) {
if (result == READER_ADD_FAILED) {
@ -1104,6 +1124,8 @@ abstract public class BrowserApp extends GeckoApp
Telemetry.HistogramAdd("FENNEC_THUMBNAILS_COUNT", BrowserDB.getCount(getContentResolver(), "thumbnails"));
} else if (event.equals("Reader:ListCountRequest")) {
handleReaderListCountRequest();
} else if (event.equals("Reader:ListStatusRequest")) {
handleReaderListStatusRequest(message.getString("url"));
} else if (event.equals("Reader:Added")) {
final int result = message.getInt("result");
final String title = message.getString("title");

View File

@ -1461,6 +1461,7 @@ abstract public class GeckoApp
//register for events
registerEventListener("log");
registerEventListener("Reader:ListCountRequest");
registerEventListener("Reader:ListStatusRequest");
registerEventListener("Reader:Added");
registerEventListener("Reader:Removed");
registerEventListener("Reader:Share");
@ -1996,6 +1997,7 @@ abstract public class GeckoApp
{
unregisterEventListener("log");
unregisterEventListener("Reader:ListCountRequest");
unregisterEventListener("Reader:ListStatusRequest");
unregisterEventListener("Reader:Added");
unregisterEventListener("Reader:Removed");
unregisterEventListener("Reader:Share");

View File

@ -62,13 +62,12 @@ public class ReaderModeUtils {
return urlFromAboutReader.equals(currentUrl);
}
public static String getAboutReaderForUrl(String url, boolean inReadingList) {
return getAboutReaderForUrl(url, -1, inReadingList);
public static String getAboutReaderForUrl(String url) {
return getAboutReaderForUrl(url, -1);
}
public static String getAboutReaderForUrl(String url, int tabId, boolean inReadingList) {
String aboutReaderUrl = "about:reader?url=" + Uri.encode(url) +
"&readingList=" + (inReadingList ? 1 : 0);
public static String getAboutReaderForUrl(String url, int tabId) {
String aboutReaderUrl = "about:reader?url=" + Uri.encode(url);
if (tabId >= 0)
aboutReaderUrl += "&tabId=" + tabId;

View File

@ -464,7 +464,7 @@ public class Tab {
Tabs.getInstance().loadUrl(ReaderModeUtils.getUrlFromAboutReader(mUrl));
} else if (mReaderEnabled) {
mEnteringReaderMode = true;
Tabs.getInstance().loadUrl(ReaderModeUtils.getAboutReaderForUrl(mUrl, mId, mReadingListItem));
Tabs.getInstance().loadUrl(ReaderModeUtils.getAboutReaderForUrl(mUrl, mId));
}
}

View File

@ -153,7 +153,7 @@ abstract class HomeFragment extends Fragment {
if (item.getItemId() == R.id.home_open_private_tab)
flags |= Tabs.LOADURL_PRIVATE;
final String url = (info.inReadingList ? ReaderModeUtils.getAboutReaderForUrl(info.url, true) : info.url);
final String url = (info.inReadingList ? ReaderModeUtils.getAboutReaderForUrl(info.url) : info.url);
Tabs.getInstance().loadUrl(url, flags);
Toast.makeText(context, R.string.new_tab_opened, Toast.LENGTH_SHORT).show();
return true;
@ -166,7 +166,7 @@ abstract class HomeFragment extends Fragment {
}
if (itemId == R.id.home_open_in_reader) {
final String url = ReaderModeUtils.getAboutReaderForUrl(info.url, true);
final String url = ReaderModeUtils.getAboutReaderForUrl(info.url);
Tabs.getInstance().loadUrl(url, Tabs.LOADURL_NONE);
return true;
}

View File

@ -101,7 +101,7 @@ public class ReadingListPage extends HomeFragment {
}
String url = c.getString(c.getColumnIndexOrThrow(URLColumns.URL));
url = ReaderModeUtils.getAboutReaderForUrl(url, true);
url = ReaderModeUtils.getAboutReaderForUrl(url);
// This item is a TwoLinePageRow, so we allow switch-to-tab.
mUrlOpenListener.onUrlOpen(url, EnumSet.of(OnUrlOpenListener.Flags.ALLOW_SWITCH_TO_TAB));

View File

@ -33,6 +33,7 @@ let AboutReader = function(doc, win) {
Services.obs.addObserver(this, "Reader:Remove", false);
Services.obs.addObserver(this, "Reader:ListCountReturn", false);
Services.obs.addObserver(this, "Reader:ListCountUpdated", false);
Services.obs.addObserver(this, "Reader:ListStatusReturn", false);
this._article = null;
@ -119,7 +120,8 @@ let AboutReader = function(doc, win) {
dump("Decoding query arguments");
let queryArgs = this._decodeQueryString(win.location.href);
this._isReadingListItem = (queryArgs.readingList == "1");
// Track status of reader toolbar add/remove toggle button
this._isReadingListItem = -1;
this._updateToggleButton();
// Track status of reader toolbar list button
@ -192,8 +194,8 @@ AboutReader.prototype = {
case "Reader:Add": {
let args = JSON.parse(aData);
if (args.url == this._article.url) {
if (!this._isReadingListItem) {
this._isReadingListItem = true;
if (this._isReadingListItem != 1) {
this._isReadingListItem = 1;
this._updateToggleButton();
}
}
@ -202,8 +204,8 @@ AboutReader.prototype = {
case "Reader:Remove": {
if (aData == this._article.url) {
if (this._isReadingListItem) {
this._isReadingListItem = false;
if (this._isReadingListItem != 0) {
this._isReadingListItem = 0;
this._updateToggleButton();
}
}
@ -211,7 +213,7 @@ AboutReader.prototype = {
}
case "Reader:ListCountReturn":
case "Reader:ListCountUpdated": {
case "Reader:ListCountUpdated": {
let count = parseInt(aData);
if (this._readingListCount != count) {
let isInitialStateChange = (this._readingListCount == -1);
@ -222,6 +224,28 @@ AboutReader.prototype = {
if (isInitialStateChange) {
this._setToolbarVisibility(true);
}
// Initial readinglist count is requested before any page is displayed
if (this._article) {
this._requestReadingListStatus();
}
}
break;
}
case "Reader:ListStatusReturn": {
let args = JSON.parse(aData);
if (args.url == this._article.url) {
if (this._isReadingListItem != args.inReadingList) {
let isInitialStateChange = (this._isReadingListItem == -1);
this._isReadingListItem = args.inReadingList;
this._updateToggleButton();
// Display the toolbar when all its initial component states are known
if (isInitialStateChange) {
this._setToolbarVisibility(true);
}
}
}
break;
}
@ -263,6 +287,7 @@ AboutReader.prototype = {
Services.obs.removeObserver(this, "Reader:Remove");
Services.obs.removeObserver(this, "Reader:ListCountReturn");
Services.obs.removeObserver(this, "Reader:ListCountUpdated");
Services.obs.removeObserver(this, "Reader:ListStatusReturn");
break;
}
},
@ -270,7 +295,7 @@ AboutReader.prototype = {
_updateToggleButton: function Reader_updateToggleButton() {
let classes = this._doc.getElementById("toggle-button").classList;
if (this._isReadingListItem) {
if (this._isReadingListItem == 1) {
classes.add("on");
} else {
classes.remove("on");
@ -291,14 +316,21 @@ AboutReader.prototype = {
gChromeWin.sendMessageToJava({ type: "Reader:ListCountRequest" });
},
_requestReadingListStatus: function Reader_requestReadingListStatus() {
gChromeWin.sendMessageToJava({
type: "Reader:ListStatusRequest",
url: this._article.url
});
},
_onReaderToggle: function Reader_onToggle() {
if (!this._article)
return;
this._isReadingListItem = !this._isReadingListItem;
this._isReadingListItem = (this._isReadingListItem == 1) ? 0 : 1;
this._updateToggleButton();
if (this._isReadingListItem) {
if (this._isReadingListItem == 1) {
gChromeWin.Reader.storeArticleInCache(this._article, function(success) {
dump("Reader:Add (in reader) success=" + success);
@ -462,7 +494,7 @@ AboutReader.prototype = {
return;
// Don't allow visible toolbar until banner state is known
if (this._readingListCount == -1)
if (this._readingListCount == -1 || this._isReadingListItem == -1)
return;
if (this._getToolbarVisibility() === visible)
@ -632,6 +664,7 @@ AboutReader.prototype = {
this._maybeSetTextDirection(article);
this._contentElement.style.display = "block";
this._requestReadingListStatus();
this._toolbarEnabled = true;
this._setToolbarVisibility(true);