Closed
Bug 1084689
Opened 10 years ago
Closed 10 years ago
Update ReadingList banner icon for dup pages in ReaderMode on longpress ReaderList pageAction
Categories
(Firefox for Android Graveyard :: Reader View, defect)
Tracking
(firefox36 verified)
VERIFIED
FIXED
Firefox 36
Tracking | Status | |
---|---|---|
firefox36 | --- | verified |
People
(Reporter: capella, Assigned: capella)
References
Details
Attachments
(1 file)
1.65 KB,
patch
|
lucasr
:
review+
Margaret
:
review+
|
Details | Diff | Splinter Review |
Long pressing the pageaction to add a page to the ReadingList fails to update the readingList-add/remove banner icon for duplicate pages already openned in other tabs that are currently displayed in ReaderMode. STR: Open the same url in two tabs Change the first tab into ReaderMode by tapping the awesomebar reader icon On the second tab, add the page to the ReadingList by long pressing the awesomebar reader icon Switch back to the first tab Observe the add/remove icon in the banner strip incorrectly indicates the page can be added to the ReadingList. If you tap the icon, you'll get "Failed to add page to your Reading List" message (as it's already in the list) ... note the icon does then update itself to the correct status (Garbage-can / page can be removed from the ReadingList).
Attachment #8507316 -
Flags: review?(lucasr.at.mozilla)
Comment 1•10 years ago
|
||
Comment on attachment 8507316 [details] [diff] [review] updateBannerIcon.diff Review of attachment 8507316 [details] [diff] [review]: ----------------------------------------------------------------- Wow, it is confusing that we create "Reader:Add" messages from both Java *and* JS. Good catch. This looks fine to me. We can let lucasr chime in if he has more to say, but he's PTO this week, so feel free to land this if he doesn't get back to you soon :)
Attachment #8507316 -
Flags: review+
Assignee | ||
Comment 2•10 years ago
|
||
try-push, cause I do that https://tbpl.mozilla.org/?tree=Try&rev=afac29e0675f
Status: NEW → ASSIGNED
Comment 3•10 years ago
|
||
Comment on attachment 8507316 [details] [diff] [review] updateBannerIcon.diff LGTM. I didn't look very deep into this but I trust margaret ;-)
Attachment #8507316 -
Flags: review?(lucasr.at.mozilla) → review+
Assignee | ||
Comment 4•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/db93f24ef8f4
Comment 5•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/db93f24ef8f4
Assignee: nobody → markcapella
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Comment 6•10 years ago
|
||
Verified as fixed in Build: Firefox for Android 36.0a1 (2014-10-24) Device: Asus Transformer Pad TF300T (Android 4.2.1)
Status: RESOLVED → VERIFIED
status-firefox36:
--- → verified
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•