Closed Bug 1157682 Opened 5 years ago Closed 5 years ago

Should ignore query string / hashes for checking for "home page"

Categories

(Toolkit :: Reader Mode, defect)

defect
Not set
Points:
2

Tracking

()

VERIFIED FIXED
mozilla40
Iteration:
40.2 - 27 Apr
Tracking Status
firefox38 --- wontfix
firefox38.0.5 --- verified
firefox39 --- verified
firefox40 --- verified

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

Attachments

(1 file, 1 obsolete file)

Status: NEW → ASSIGNED
Iteration: --- → 40.2 - 27 Apr
Points: --- → 2
Flags: qe-verify+
Flags: in-testsuite-
Flags: firefox-backlog+
Attached file MozReview Request: bz://1157682/Gijs (obsolete) —
/r/7497 - Bug 1157682 - ignore querystring / hash when determining 'home page' for reader mode, r?margaret

Pull down this commit:

hg pull -r 6ae1fd9e91389b3699515006016247b6ee6f5c2b https://reviewboard-hg.mozilla.org/gecko/
Attachment #8596584 - Flags: review?(margaret.leibovic)
As a side effect, this will also fix bug 1151087, at least for bing.
Comment on attachment 8596584 [details]
MozReview Request: bz://1157682/Gijs

https://reviewboard.mozilla.org/r/7495/#review6285

::: toolkit/components/reader/ReaderMode.jsm:250
(Diff revision 1)
> +    } catch (ex) { /* ignore if this doesn't work */ }

When would this not work? If `uri` can't be turned into an `nsIURL`, maybe we should just return false, since I would assume it isn't a well formatted URL.
Attachment #8596584 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8596584 [details]
MozReview Request: bz://1157682/Gijs

Approval Request Comment
[Feature/regressing bug #]: reader mode
[User impact if declined]: reader mode offered on pages where it shouldn't be
[Describe test coverage new/current, TreeHerder]: not for this specific part
[Risks and why]: pretty low, restricted to how/when we offer reader mode
[String/UUID change made/needed]: no.
Attachment #8596584 - Flags: approval-mozilla-release?
Attachment #8596584 - Flags: approval-mozilla-beta?
Attachment #8596584 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/836194cdafc4
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Comment on attachment 8596584 [details]
MozReview Request: bz://1157682/Gijs

We are late in the beta cycle. I would like to take it only for 38.0.5.
Attachment #8596584 - Flags: approval-mozilla-release?
Attachment #8596584 - Flags: approval-mozilla-release-
Attachment #8596584 - Flags: approval-mozilla-beta?
Attachment #8596584 - Flags: approval-mozilla-beta+
Attachment #8596584 - Flags: approval-mozilla-aurora?
Attachment #8596584 - Flags: approval-mozilla-aurora+
QA Contact: andrei.vaida
Verified fixed on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.9.5 using latest Nightly 40.0a1 (buildID: 20150511030203), latest Aurora 39.0a2 (buildID: 20150511004005) and Firefox 38.0.5 Beta 1 (buildID: 20150510205200).
Attachment #8596584 - Attachment is obsolete: true
Attachment #8620133 - Flags: review+
You need to log in before you can comment on or make changes to this bug.