1222792 - Refresh loop on pinned website that was in Reader View, after Firefox restart

Status: RESOLVED FIXED

(Toolkit :: Reader Mode, defect)

Description

[Iakovos Gurulian]

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:42.0) Gecko/20100101 Firefox/42.0
Build ID: 20151102093730

Steps to reproduce:

I put a pinned website in Reader View and restarted Firefox


Actual results:

After restarting the browser, the pinned page keeps refreshing until I close it. I also cannot refresh it or exit the Reader View.


Expected results:

Either not fall in a refresh loop or, after restart, the webpage to be on normal mode instead of Reader View.

Comment 1

[YF (Yang)]

WFM on Fx43 Win10.
Test page: https://blog.mozilla.org/addons/2015/11/20/signing-api-now-available/


Could you try Firefox without add-ons?

Comment 2

[Iakovos Gurulian]

I tried it with that website but does not occur even with add-ons enabled. This website caused the problem to me: http://pitsirikos.net/2012/10/%CF%80%CF%8E%CF%82-%CF%83%CF%84%CF%81%CF%8E%CE%B8%CE%B7%CE%BA%CE%B5-%CE%BF-%CE%B4%CF%81%CF%8C%CE%BC%CE%BF%CF%82-%CE%B3%CE%B9%CE%B1-%CF%84%CE%BF%CE%BD-%CF%86%CE%B1%CF%83%CE%B9%CF%83%CE%BC%CF%8C/

I can reproduce it with or without add-ons enabled.

Comment 3

[Loic]

I can reproduce it.

STR:
1) Open http://pitsirikos.net/2012/10/%CF%80%CF%8E%CF%82-%CF%83%CF%84%CF%81%CF%8E%CE%B8%CE%B7%CE%BA%CE%B5-%CE%BF-%CE%B4%CF%81%CF%8C%CE%BC%CE%BF%CF%82-%CE%B3%CE%B9%CE%B1-%CF%84%CE%BF%CE%BD-%CF%86%CE%B1%CF%83%CE%B9%CF%83%CE%BC%CF%8C/ in Reader Mode
2) Pin it
3) Restart Firefox

Comment 4

[Loic]

It regressed by bug 1195976:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=d838f6278e74831fd357446978501edee5413c75&tochange=1d0a9ae2715d55a4852de88ad756f6efb1c363be

Comment 5

[:Gijs (he/him)]

43 is shipping this week, RC builds have been built already, and this isn't going to block it shipping. I also seem to recall this is a duplicate, but I can't find it offhand. Going to leave needinfo and hopefully find time to look at this tomorrow.

Comment 6

[:Gijs (he/him)]

Attachment: MozReview Request: Bug 1222792 - fix comparing escaped URIs in reader mode when detecting redirects, r?margaret

Bug 1222792 - fix comparing escaped URIs in reader mode when detecting redirects, r?margaret

Comment 8

[:Margaret Leibovic]

https://reviewboard.mozilla.org/r/28005/#review25203

Comment 9

[:Gijs (he/him)]

Comment on attachment 8698456 [details]
MozReview Request: Bug 1222792 - fix comparing escaped URIs in reader mode when detecting redirects, r?margaret

mm, mozreview is being wonky (bug 1232287) so manually carrying r+.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/ed92012dbd5c

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ed92012dbd5c

Comment 12

[:Gijs (he/him)]

Comment on attachment 8698456 [details]
MozReview Request: Bug 1222792 - fix comparing escaped URIs in reader mode when detecting redirects, r?margaret

Approval Request Comment
[Feature/regressing bug #]: bug 1195976
[User impact if declined]: some pages go into an infinite redirect loop if you try to view them in reader mode
[Describe test coverage new/current, TreeHerder]: sadly, the test coverage here is pretty limited - but there is some core automated testing that would have failed if this broke reader mode completely
[Risks and why]: very low, essentially making sure URL redirection code deals with escaping better than it did
[String/UUID change made/needed]: no

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

lakovos, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

Comment 14

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

I don't feel the need to track it. Will uplift to Beta44 soon anyway.

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8698456 [details]
MozReview Request: Bug 1222792 - fix comparing escaped URIs in reader mode when detecting redirects, r?margaret

Seems safe to uplift to Beta44 and Aurora45.

Comment 16

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

https://hg.mozilla.org/releases/mozilla-beta/rev/3454c50e4fb3

Comment 17

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/9d71fae90c12

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/3454c50e4fb3