Closed Bug 458565 Opened 11 years ago Closed 11 years ago

Location bar should encode parentheses on copy

Categories

(Firefox :: Address Bar, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 3.1b2

People

(Reporter: from.bmo.18, Assigned: dao)

References

()

Details

Attachments

(1 file)

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)
Hah! Bugzilla just demonstrated my point on automatic hyperlinkers. :)
Status: UNCONFIRMED → NEW
Ever confirmed: true
(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?
(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.
Depends on: 440075
Attached patch patchSplinter Review
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #341833 - Flags: review?(gavin.sharp)
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?
Attachment #341833 - Flags: review?(gavin.sharp) → review+
(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.
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/7f66adec37ac
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.1b2
Flags: in-litmus?
in-litmus+:

https://litmus.mozilla.org/show_test.cgi?id=7048
Flags: in-litmus? → in-litmus+
Confirming fixed. Thanks!
Status: RESOLVED → VERIFIED
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.
Depends on: 824887
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.
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
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?)
Flags: needinfo?(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.
Flags: needinfo?(dao)
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.
Can you explain why this behavior is problematic in more detail? Ideally in a new bug report, feel free to CC me.
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!
Depends on: 501719
No longer depends on: 501719
You need to log in before you can comment on or make changes to this bug.