Closed Bug 1272340 Opened 5 years ago Closed 5 years ago

about:reader URL is displayed in Reader View


(Firefox for Android Graveyard :: Reader View, defect)

48 Branch
Not set


(firefox47 unaffected, firefox48 verified, firefox49 verified, fennec48+)

Firefox 49
Tracking Status
firefox47 --- unaffected
firefox48 --- verified
firefox49 --- verified
fennec 48+ ---


(Reporter: TeoVermesan, Assigned: sebastian)


(Keywords: regression)


(3 files)

Steps to reproduce:
1. Go to
2. Choose an article and enter reader view

Expected results:
- The main domain (origin) for long URLs should be displayed in URL Bar

Actual results:
- "about:reader?url=https%.." is displayed

10-05 not affected
11-05 affected

tracking-fennec: --- → ?
This could be caused by me and bug 1269832.
(In reply to Sebastian Kaspari (:sebastian) from comment #2)
> Yep, this should be 'strippedURL':
> mozilla/gecko/toolbar/

Actually there's no reason for the last else block - introduced in bug 1269832: updateAndColorTitleFromFullURL() handles cases with/without base domain and different protocols just well.
This patch changes two things:
* Check if the URL is http/https after stripping the about:reader URL.
* Always call updateAndColorTitleFromFullURL() as fallback for URL formatting (like in previous versions)

Review commit:
See other reviews:
Attachment #8752134 - Flags: review?(margaret.leibovic)
Comment on attachment 8752134 [details]
MozReview Request: Bug 1272340 - ToolbarDisplayLayout: Handle about:reader URLs. r=margaret

::: mobile/android/base/java/org/mozilla/gecko/toolbar/
(Diff revision 1)
> -        final boolean isHttpOrHttps = StringUtils.isHttpOrHttps(url);
>          final String baseDomain = tab.getBaseDomain();
>          String strippedURL = stripAboutReaderURL(url);
> +        final boolean isHttpOrHttps = StringUtils.isHttpOrHttps(strippedURL);

This is kinda weird because technically the connection is always secure on a reader view page because the content is hosted in the browser. But I guess this does indicate whether or not the original content was downloaded over http vs. https, so it makes sense to display it this way.
Attachment #8752134 - Flags: review?(margaret.leibovic) → review+
tracking-fennec: ? → 48+
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Want to request uplift to aurora 48 since this is marked as a regression from 48?
Flags: needinfo?(s.kaspari)
Attached image Pp2Tds-w.jpg
Verified as fixed using:
Device: ONE A2001 (Android 5.1.1)
Build: Firefox for Android 49.0a1(2016-05-24)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #8)
> Want to request uplift to aurora 48 since this is marked as a regression
> from 48?

Yeah, this is the updated patch for Aurora.

Approval Request Comment
[Feature/regressing bug #]: Regression introduced in bug 1269832.
[User impact if declined]: about:reader is shown in URL bar.
[Describe test coverage new/current, TreeHerder]: Manual testing + Patch is in Nightly for a week now.
[Risks and why]: Low - patch is tested in Nightly and it's using the code that was in use on tablets already.
[String/UUID change made/needed]: -
Flags: needinfo?(s.kaspari)
Attachment #8757914 - Flags: approval-mozilla-aurora?
Comment on attachment 8757914 [details] [diff] [review]

Regression, taking it
Attachment #8757914 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed using:
Device: Nexus 7 (Android 5.1.1)
Build: Firefox for Android 48.0a2(2016-05-31)
Version: unspecified → 48 Branch
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.