Closed
Bug 1160556
Opened 9 years ago
Closed 9 years ago
Unrestored Reader View tabs display the about:reader path rather than a title
Categories
(Toolkit :: Reader Mode, defect)
Toolkit
Reader Mode
Tracking
()
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: caspy77, Assigned: ttaubert)
References
Details
Attachments
(1 file)
3.48 KB,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
After a restart, we want unrestored tabs to look like normal tabs, but if the tab was in reader view at the time of the restart, instead of the original title the about:reader path is displayed. This looks messy and can be confusing to users.
Comment 1•9 years ago
|
||
I suspect this is because we use JS to set the reader view title after we get the article: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/reader/AboutReader.jsm?force=1#775 However, I would think that session restore would just try to restore the old title... maybe we have this functionality disabled for about: pages? ttaubert, do you know what's going on here?
Flags: needinfo?(ttaubert)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → ttaubert
Iteration: --- → 40.3 - 11 May
Points: --- → 2
Flags: qe-verify+
Flags: needinfo?(ttaubert)
Flags: firefox-backlog+
Assignee | ||
Updated•9 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•9 years ago
|
||
Rather trivial patch. Until now we didn't invalidate shistory data when the page title changed. Doesn't happen very often so no one noticied.
Attachment #8601542 -
Flags: review?(gijskruitbosch+bugs)
Comment 4•9 years ago
|
||
Comment on attachment 8601542 [details] [diff] [review] 0001-Bug-1160556-Recollect-session-history-data-when-the-.patch Review of attachment 8601542 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/sessionstore/test/browser_page_title.js @@ +29,5 @@ > + content.document.title = "new title"; > + }); > + > + // Remove the tab. > + yield promiseRemoveTab(tab); Please rename this to something that says what it does. Might be a followup and have this just add a comment.
Attachment #8601542 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #4) > > + // Remove the tab. > > + yield promiseRemoveTab(tab); > > Please rename this to something that says what it does. Might be a followup > and have this just add a comment. Added a comment to the function definition in the head.js file.
https://hg.mozilla.org/mozilla-central/rev/d6e5fb263125
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Updated•9 years ago
|
QA Contact: andrei.vaida
Comment 8•9 years ago
|
||
I am unable to reproduce this issue with Nightly from 2015-05-01 under Windows 8.1 32-bit and Ubuntu 12.04 64-bit - the correct title was displayed. Could you please provide some guidance in order to properly verify this fix? Thanks in advance!
Flags: needinfo?(ttaubert)
Assignee | ||
Comment 9•9 years ago
|
||
IIRC this wasn't too easy to reproduce and I did that only by writing a test. We have a test, so maybe we don't need manual QA here?
Flags: needinfo?(ttaubert) → in-testsuite+
Comment 10•9 years ago
|
||
(In reply to Tim Taubert [:ttaubert] from comment #9) > IIRC this wasn't too easy to reproduce and I did that only by writing a > test. We have a test, so maybe we don't need manual QA here? I agree with the above, and setting as qe-verify- based on this. Thank you for your feedback Tim!
Flags: qe-verify+ → qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•