1257804 - Copying "normal" URL from location bar when in reader mode broke (now includes "about:reader?url=" and escapes the real URL)

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[:Gijs (he/him)]

Noticed this today when reviewing narrator changes.

This is a regression from bug 824887. When we implemented reader mode in desktop, we made sure copying the URL just copied the "regular" URL instead of the bit including "about:reader?url=" and the escaped copy of the URL.

It looks like this somehow broke in bug 824887.

[Tracking Requested - why for this release]:
regression for 47

Comment 1

[:Gijs (he/him)]

Attachment: MozReview Request: Bug 1257804 - avoid copy-pasting about:reader and copy-paste the URL instead, r?dolske

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

Comment 2

[:Gijs (he/him)]

https://reviewboard.mozilla.org/r/41379/#review37853

::: browser/base/content/urlbarBindings.xml:615
(Diff revision 1)
> +          // Avoid copying 'about:reader?url=', and always provide the original URI:
> +          let readerOriginalURL = ReaderMode.getOriginalUrl(uri.spec);
> +          if (readerOriginalURL) {
> +            uri = uriFixup.createFixupURI(readerOriginalURL, Ci.nsIURIFixup.FIXUP_FLAG_NONE);
> +          }

Could keep this strictly within the pageproxystate == "valid" case in the if statement above this, but this seemed easier to read, more generic, and more futureproof.

Comment 3

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

New regression in Fx47, tracked.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8732803 [details]
MozReview Request: Bug 1257804 - avoid copy-pasting about:reader and copy-paste the URL instead, r?dolske

https://reviewboard.mozilla.org/r/41379/#review39163

Comment 6

[Pulsebot]

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

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/48d54c93dcf3

Comment 8

[:Gijs (he/him)]

Comment on attachment 8732803 [details]
MozReview Request: Bug 1257804 - avoid copy-pasting about:reader and copy-paste the URL instead, r?dolske

Approval Request Comment
[Feature/regressing bug #]: bug 824887
[User impact if declined]: about:reader shows up in URLs when copied from the URL bar while in reader mode
[Describe test coverage new/current, TreeHerder]: patch has a test to avoid re-regressing this
[Risks and why]: pretty low, the escaping functionality of the URL bar is fairly well-tested generally
[String/UUID change made/needed]: nope.

Comment 9

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

Comment on attachment 8732803 [details]
MozReview Request: Bug 1257804 - avoid copy-pasting about:reader and copy-paste the URL instead, r?dolske

Recent regression, Aurora47+

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3e3bdbc7ac12

Comment 11

[Cornel Ionce [:noni] [Hubs QA]]

Reproduced this issue on 2016-03-18 Nightly (build ID 20160318030236) on Windows 10 x64.

Confirming the fix across platforms using:
* Firefox 47.0b1, build ID 20160425095909
* Latest 48.0a2 Aurora, build ID 20160425101338.