458565 - Location bar should encode parentheses on copy

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[Daniel Luz]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20081004 Minefield/3.1b1pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20081004 Minefield/3.1b1pre

The location bar encodes certain special characters (which are displayed unencoded), and should be part of them.

Parentheses in URLs often confuse automatic hyperlinkers and lightweight markup languages. Many "smart" linkers will automatically ignore a closing parenthesis on the end of a detected URL, and some languages, like Markdown, even use them as part of their hyperlinking syntax. Meanwhile, Wikipedia has popularized addresses containing parentheses, and many browsers copy those parentheses unencoded when you highlight the URL in the location bar and copy it.
The combination of these factors can be seen somewhat regularly on forums and blogs, pointing to invalid links like "Firefox (disambiguation", leaving the closing parenthesis outside of the linked address.

Reproducible: Always

Steps to Reproduce:
1. Go to a website URL containing parenthesis.
Example: http://en.wikipedia.org/wiki/Firefox_(disambiguation)
2. Highlight the URL in the location bar, and copy it.
3. Paste it on a text field.
Actual Results:  
The parentheses are copied unencoded:
http://en.wikipedia.org/wiki/Firefox_(disambiguation)

Expected Results:  
The parentheses should be encoded to play nicely with lightweight markups:
http://en.wikipedia.org/wiki/Firefox_%28disambiguation%29

(while still being *displayed* as parentheses)

Comment 1

[Daniel Luz]

Hah! Bugzilla just demonstrated my point on automatic hyperlinkers. :)

Comment 2

[Dão Gottwald [:dao]]

(In reply to comment #0)
> Parentheses in URLs often confuse automatic hyperlinkers and lightweight markup
> languages. Many "smart" linkers will automatically ignore a closing parenthesis
> on the end of a detected URL,

So how about encoding the trailing closing parenthesis only:
http://en.wikipedia.org/wiki/x(foo)(bar%29

> and some languages, like Markdown, even use them
> as part of their hyperlinking syntax.

Would a parenthesis in the middle of the address confuse the Markdown parser?

Comment 3

[Daniel Luz]

(In reply to comment #2)
> So how about encoding the trailing closing parenthesis only:
> http://en.wikipedia.org/wiki/x(foo)(bar%29

That certainly works, at least for the more common cases. I'm just bothered by the lack of symmetry, and IMHO this is more than just superficial aesthetics; seeing a bunch of open parentheses and no closing ones could potentially confuse visual scanning.

> > and some languages, like Markdown, even use them
> > as part of their hyperlinking syntax.
> 
> Would a parenthesis in the middle of the address confuse the Markdown parser?

Yes, because it delimits the URL by a closing parenthesis:
[Example](http://www.example.org/)

I've never seen a Markdown parser choke on an opening parenthesis, but any closing one is guaranteed to cause problems due to its syntax.

Comment 4

[Dão Gottwald [:dao]]

Attachment: patch

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 341833 [details] [diff] [review]
patch

>diff --git a/browser/base/content/urlbarBindings.xml b/browser/base/content/urlbarBindings.xml

>+              // Parentheses are known to confuse third-party applications (bug 458565).
>+              val = val.replace(/[()]/g, function (c) escape(c));

You can just pass escape directly, right?

Comment 6

[Dão Gottwald [:dao]]

(In reply to comment #5)
> >+              val = val.replace(/[()]/g, function (c) escape(c));
> 
> You can just pass escape directly, right?

replace passes multiple arguments and escape accepts two, which causes failure.

Comment 7

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/7f66adec37ac

Comment 8

[Stephen Donner [:stephend] Not actively reading bugmail]

in-litmus+:

https://litmus.mozilla.org/show_test.cgi?id=7048

Comment 9

[Daniel Luz]

Confirming fixed. Thanks!

Comment 10

[Saair Quaderi]

This was not a bug with firefox, but now the "fix" has produced one.

This was essentially an issue with users of a third party application not knowing to escape close parentheses as specified by a third party syntax.

The "fix" now takes VALID urls with parentheses in them and unwittingly for the person who is copying the url, copies in a different BROKEN url that relies on the hope that either the browser or the website will somehow be able to deal with the incorrect url.

This behavior is not only inconsistent with what is shown in the url bar, but is also inconsistent with the standards and the behavior of every other major browser. No other browsers arbitrarily single out parentheses to incorrectly encode them only when copying.

Comment 11

[unorthodox.engineers]

So it was YOU!

Yes, thanks for that "Fix". It completely derailed a project, cost me two weeks of work, and another week here on bugzilla trying to convince people that the RFC specifically treats brackets as characters with special meaning. (On par with the ":" and "/" characters.)

https://bugzilla.mozilla.org/show_bug.cgi?id=824887

Perhaps we should read the spec a little more closely in future before making arbitrary changes that create convenience for random third-party apps, and have the unintended consequence of breaking the internet.

Comment 12

[unorthodox.engineers]

Just to be clear, according to RFC3987, the brackets characters are absolutely NOT interchangeable with percent-encoded 'equivalents', any more so than encoding the "://" at the start of the URL or the 
"?" in the middle. They are part of the "reserved character set" intended for syntax _within_ URLs.

Any use of brackets as _data_ must be percent escaped. Any use as part of the _URL syntax_ must NOT be. Same as the rules for ":" or "?" characters in filenames within the url.  

There was also a long discussion over here:

https://support.mozilla.org/en-US/questions/945166

Comment 13

[hdfssk]

Good idea or bad, there are use cases where percent-encoding parenthesis like this is a problem.

An about:config entry to turn this behavior off seems like a good compromise solution. Presiding devs: would a patch to add one be accepted? Is this bug the right forum to discuss implementing that (and if not here, where?)

Comment 14

[Dão Gottwald [:dao]]

(In reply to hdfssk from comment #13)
> Good idea or bad, there are use cases where percent-encoding parenthesis
> like this is a problem.
> 
> An about:config entry to turn this behavior off seems like a good compromise
> solution. Presiding devs: would a patch to add one be accepted?

Likely not. If percent-encoding parenthesis is a problem, then fixing it behind a hidden pref seems like the wrong solution.

> Is this bug the right forum to discuss implementing that

No.

> (and if not here, where?)

The firefox-dev mailing list.

Comment 15

[Saair Quaderi]

Dão, an we please, please, please undo this change to the url copying functionality? It is clearly buggy and not standards compliant and is causing a lot of issues.

What can I do to get this fixed? I feel like my hands are completely tied.

Eliminating this bug from Firefox requires so few changes to the code, and yet I've been stuck having to tell thousands of users not to use it when accessing various sites (despite Firefox being my personally preferred browser) because the alternative is dealing with users complaining about a proliferation of broken links.

Comment 16

[:Gavin Sharp [email: [e-mail]]]

Can you explain why this behavior is problematic in more detail? Ideally in a new bug report, feel free to CC me.

Comment 17

[Jay R O]

As @hdfssk proposed, can we PLEASE have an about:config entry to turn this behavior off?

All other browsers do not behave this way, it's quite problematic when copy & pasting having to manually change escaped characters back into parentheses.  PLEASE!