1292903 - Fennec crashes with about:reader;https://www.1843magazine.com/features/minds-turned-to-ash

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Jeff Muizelaar [:jrmuizel]]

STR:
Load https://www.1843magazine.com/features/minds-turned-to-ash
append 'about:reader;' to https://www.1843magazine.com/features/minds-turned-to-ash and load that.
Fennec crashes.


https://crash-stats.mozilla.com/report/index/1b012fdf-fe30-4974-b4e9-2b1262160806

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

@ahunt: Can you take a look? I assume that stripAboutReaderURL() doesn't work correctly if some parts of an about:reader URL are missing.

Comment 2

[nemo]

Hey.  Just for some context, this is one of those irritating sites that covers the article text with a CSS blank box if one has JS enabled (which I've been increasingly doing on mobile due to 3rd party advertiser tracking, hostile 3rd party code, and to save bandwidth/battery).

So I thought I'd try typing in the about:reader url manually since there's no way to force it on for a website.

I need to look into addons to see if there's an addon to force reader mode, or possibly just force off CSS for a website.   Given this article is perfectly readable in w3m, I'm guessing forcing CSS off would do the trick.

Comment 3

[nemo]

er... if one has JS *disabled* ☺

Comment 4

[Andrzej Hunt :ahunt]

Wow, we definitely shouldn't crash in this case, I'll try to fix this.

JFYI the expected syntax is about:reader?url=URL.

Comment 5

[Andrzej Hunt :ahunt]

Attachment: Bug 1292903 - Use stripAboutReaderUrl in ToolbarDisplayLayout to avoid crashing on malformed about:reader URLs

Returning null will also cause crashes down the line, so this is a win-win.

Review commit: https://reviewboard.mozilla.org/r/70418/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70418/

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8779418 [details]
Bug 1292903 - Use stripAboutReaderUrl in ToolbarDisplayLayout to avoid crashing on malformed about:reader URLs

https://reviewboard.mozilla.org/r/70418/#review67686

::: mobile/android/base/java/org/mozilla/gecko/reader/ReaderModeUtils.java:15
(Diff revision 1)
>  import android.net.Uri;
>  
>  public class ReaderModeUtils {
>      private static final String LOGTAG = "ReaderModeUtils";
>  
>      public static String getUrlFromAboutReader(String aboutReaderUrl) {

It looks like this is exactly what stripAboutReaderUrl() is doing. Maybe we should just call this one from ToolbarDisplayLayout?

Comment 7

[Andrzej Hunt :ahunt]

Attachment: Bug 1292903 - Add comment explaining getUrlFromAboutReader

Review commit: https://reviewboard.mozilla.org/r/70724/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70724/

Comment 8

[Andrzej Hunt :ahunt]

Comment on attachment 8779418 [details]
Bug 1292903 - Use stripAboutReaderUrl in ToolbarDisplayLayout to avoid crashing on malformed about:reader URLs

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70418/diff/1-2/

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8779418 [details]
Bug 1292903 - Use stripAboutReaderUrl in ToolbarDisplayLayout to avoid crashing on malformed about:reader URLs

https://reviewboard.mozilla.org/r/70418/#review68070

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8779793 [details]
Bug 1292903 - Add comment explaining getUrlFromAboutReader

https://reviewboard.mozilla.org/r/70724/#review68072

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ad9a908a7489
Use stripAboutReaderUrl in ToolbarDisplayLayout to avoid crashing on malformed about:reader URLs r=sebastian
https://hg.mozilla.org/integration/autoland/rev/808e3ed40d6c
Add comment explaining getUrlFromAboutReader r=sebastian

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ad9a908a7489
https://hg.mozilla.org/mozilla-central/rev/808e3ed40d6c

Comment 13

[Teodora Vermesan (:TeoVermesan)]

Verified as fixed using:
Device: One A2001 (Android 5.1.1)
Build: Firefox for Android 51.0a1 (2016-08-18)
I can reproduce the crash on latest Aurora and Firefox for Android 48.0.
On Firefox for Android 49 Beta 4, after adding about:reader to the link, Firefox becomes unresponsive, but does not crash:http://i.imgur.com/gEDoHCK.png 
As you can see, the three dot menu is hidden. Tapping the URL Bar does nothing.