Refresh cached reader-view content when updated original page is available

NEW
Unassigned

Status

()

Firefox for Android
Reader View
P5
enhancement
2 years ago
10 months ago

People

(Reporter: Vincent, Unassigned, Mentored)

Tracking

48 Branch
All
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [lang=js][good next bug])

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:48.0) Gecko/20100101 Firefox/48.0
Build ID: 20160728204513

Steps to reproduce:

1. Visited a page (in this case, the mobile read-only view of a Google Docs document).
2. Opened reading mode
3. Bookmarked the page in reading mode to save it for offline use
4. Update the page (in this case, I edited the document on my computer)
5. Visit the page again, seeing the updated content.
6. Open reader view.


Actual results:

I still saw the version of the page as it was when I first saved it for offline use.


Expected results:

Having seen the updated content in normal view, reader view should update with the new content as well.

I also removed the bookmark and then re-added it, then re-opened reader view. Still, the reader view was not updated.
(Reporter)

Updated

2 years ago
OS: Unspecified → Android
Version: unspecified → 48 Branch

Comment 1

2 years ago
Andrzej, is this something we can control/update?
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(ahunt)
Hardware: Unspecified → All
Do we know if this is a regression?

Comment 3

2 years ago
Bookmarking a reader-view page is currently equivalent to saving the reader-view page in our offline cache. We also don't do any type of cache invalidation.

This isn't really a regression: previously it was impossible to even bookmark reader view pages, and saving a page to the defunct reading list would result in exactly the same issue (with the exception of those cases where caching didn't work, where we always have to load the page from the internet).

There are probably a few ways we could "fix" this:
- Load the original website (instead of using cache) when connectivity is available: this is probably very tricky to implement, and is likely to be confusing from a UX perspective.
- Refresh offline pages periodically: probably even more confusing if you can't predict when an update will happen.
- Make offline caching independent of bookmarking reader-view pages: the UX for this gets even more complicated.
- Special case entering reader-view from the RV button (vs. opening it from bookmarks), and overwrite the cached article in this case?
Flags: needinfo?(ahunt)

Comment 4

2 years ago
(In reply to Vincent from comment #0)
> 
> I also removed the bookmark and then re-added it, then re-opened reader
> view. Still, the reader view was not updated.

I missed this part: this sounds like a definite bug: the cache should definitely be cleared when removing a reader-view bookmark. It sounds like:
1) bookmark was removed, but the cached item wasn't removed
2) bookmark of web (non-readerview) version of the page was added
3) entering reader-view accessed the cached version (since the cached is keyed by URL)

Comment 5

2 years ago
I'll dig into clearing cached items as soon as they're no longer needed as part of this bug. I think any other changes might require a UX rethink though, I'm not sure that's likely to happen anytime soon.
Assignee: nobody → ahunt

Comment 6

2 years ago
(In reply to Andrzej Hunt :ahunt from comment #5)
> I'll dig into clearing cached items as soon as they're no longer needed as
> part of this bug. I think any other changes might require a UX rethink
> though, I'm not sure that's likely to happen anytime soon.

I've debugged this somewhat and I've not found any scenarios where cache clearing doesn't happen.

The two codepaths are:
- Removal of bookmark via the menu: this calls Tab.removeBookmark(), which clears the cache
- Removal of bookmark from the bookmarks panel context-menu: this calls HomeFragment.RemoveItemByURLTask.removeBookmark()

Both of these call into ReadingListHelper.removeCachedReaderItem() which successfully calls into the JS code which actual deals with cache removal (and I've tested while offline to ensure that the cache removal has succeeded).

Comment 7

2 years ago
Unassigning for now since we're focussing on AS, and I'm not sure how big a priority this is (reader view was primarily intended for reading articles, and google docs seems somewhat of an edge-case). I'm happy to continue with the bug if it's deemed important enough.

Fixing this issue would require digging into our ReaderView and general browser.js code, to implement one of the solutions in comment 3:
https://bugzilla.mozilla.org/show_bug.cgi?id=1298347#c3

I think the last option makes the most sense:
- If we enter readerview from the "normal" website version, we usually take the in-memory document content and readerify it.
- Speculation: ReaderView finds cached content, discards the in-memory version, loads the cached content (bookmarked reader view items automatically land in the cache)
- Instead we should ignore (and ideally overwrite) the cached content if we have the (non-reader) document in-memory.

Fixing this will probably require stepping through Reader.js, ReaderMode.jsm, and possibly the mobile content.js and AboutReader.jsm.
Assignee: ahunt → nobody
Mentor: ahunt
Component: Reading List → Reader View
Summary: Bookmarked reader view (i.e. saved for offline) doesn't update when the original page refreshes → Refresh cached reader-view content when updated original page is available
Whiteboard: [lang=js][good next bug]
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.