Closed Bug 1138628 Opened 5 years ago Closed 5 years ago

Don't try to find article in reader mode cache on desktop until we actually store articles in cache

Categories

(Toolkit :: Reader Mode, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: Margaret, Assigned: Margaret)

References

Details

Attachments

(1 file, 1 obsolete file)

For Fx38, I don't think we're going to store article content in the cache on desktop, so we should skip this step to look in the cache:
http://mxr.mozilla.org/mozilla-central/source/browser/modules/ReaderParent.jsm#180
But before I write a patch... Unfocused, am I correct here? Are we not going to use the cache?

There's not that much harm in having this check here, but it is an unnecessary file look-up if we're never going to find anything.
Flags: needinfo?(bmcbride)
(In reply to :Margaret Leibovic from comment #1)
> But before I write a patch... Unfocused, am I correct here? Are we not going
> to use the cache?

Correct, desktop won't be using a persisted cache for the foreseeable future.
Flags: needinfo?(bmcbride)
/r/4643 - Bug 1138628 - Don't try to find article in reader mode cache on desktop because we don't actually store them. r=Unfocused

Pull down this commit:

hg pull review -r 1bfd153456b7f7bc4d3f9ea7edfc978ceec3530e
Attachment #8572753 - Flags: review?(bmcbride)
Comment on attachment 8572753 [details]
MozReview Request: bz://1138628/margaret

https://reviewboard.mozilla.org/r/4641/#review3851

Ship It!
Attachment #8572753 - Flags: review?(bmcbride) → review+
P2 based on this apparently happening while loading pages (and causing disk io). Yay that we've already got a patch. :)
Priority: -- → P2
https://hg.mozilla.org/mozilla-central/rev/55a98ebede65
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Margaret, is there anything manual QA can help with here? If so, please set the qe-verify flag accordingly and provide a few guidelines in terms of how this fix can be verified.
Flags: qe-verify?
Flags: needinfo?(margaret.leibovic)
(In reply to Andrei Vaida, QA [:avaida] from comment #9)
> Margaret, is there anything manual QA can help with here? If so, please set
> the qe-verify flag accordingly and provide a few guidelines in terms of how
> this fix can be verified.

Nope, nothing to do here beyond making sure reader view isn't broken :)
Flags: needinfo?(margaret.leibovic)
Flags: qe-verify? → qe-verify-
Attachment #8572753 - Attachment is obsolete: true
Attachment #8619636 - Flags: review+
You need to log in before you can comment on or make changes to this bug.