Last Comment Bug 666717 - URL bar highlighting treats URLs with hyphenated schemes (e.g. view-source) as scheme-less
: URL bar highlighting treats URLs with hyphenated schemes (e.g. view-source) a...
Status: RESOLVED FIXED
: regression
Product: Firefox
Classification: Client Software
Component: Location Bar (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 7
Assigned To: Dão Gottwald [:dao]
:
Mentors:
Depends on: 1250662 668753
Blocks: 665580
  Show dependency treegraph
 
Reported: 2011-06-23 12:49 PDT by Dão Gottwald [:dao]
Modified: 2016-02-23 13:31 PST (History)
4 users (show)
dao+bmo: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (3.42 KB, patch)
2011-06-23 13:33 PDT, Dão Gottwald [:dao]
no flags Details | Diff | Review
patch (3.12 KB, patch)
2011-06-23 13:35 PDT, Dão Gottwald [:dao]
gavin.sharp: review+
Details | Diff | Review

Description Dão Gottwald [:dao] 2011-06-23 12:49:46 PDT

    
Comment 1 Dão Gottwald [:dao] 2011-06-23 13:33:07 PDT
Created attachment 541481 [details] [diff] [review]
patch

I took the list of allowed characters from <http://en.wikipedia.org/wiki/URI_scheme#Generic_syntax>.
Comment 2 Dão Gottwald [:dao] 2011-06-23 13:35:52 PDT
Created attachment 541484 [details] [diff] [review]
patch

Err, instead of changing tragetValue to targetValue, I did it the other way around...
Comment 3 Daniel Cater 2011-06-23 17:00:43 PDT
Thanks for fixing this quickly.

let protocol = value.match(/^[a-z\d.+-]+:/);

I think this would be clearer with the special characters .+- escaped (\.\+\-). It has the same effect but it looks odd (to me at least) to have an unescaped . that is actually a literal and +- suggests the start of a character range.

I think that a line should be added to browser_urlbarTrimURLs.js test like:

testVal("view-source:http://www.mozilla.org/");

And maybe another nested scheme like:

testVal("jar:http://www.mozilla.org/example.jar!/");

could be added to both that test and the highlighting test (unless you plan to have the domain highlighted in that case).
Comment 4 Dão Gottwald [:dao] 2011-06-23 22:10:37 PDT
> I think this would be clearer with the special characters .+- escaped
> (\.\+\-). It has the same effect but it looks odd (to me at least) to have
> an unescaped . that is actually a literal and +- suggests the start of a
> character range.

I escaped the hyphen, since it actually does have a special meaning in character classes, e.g. .-+ wouldn't work.

> I think that a line should be added to browser_urlbarTrimURLs.js test like:
> 
> testVal("view-source:http://www.mozilla.org/");
> 
> And maybe another nested scheme like:
> 
> testVal("jar:http://www.mozilla.org/example.jar!/");
> 
> could be added to both that test and the highlighting test (unless you plan
> to have the domain highlighted in that case).

done
Comment 5 Dão Gottwald [:dao] 2011-06-23 22:10:58 PDT
http://hg.mozilla.org/mozilla-central/rev/e0b805cab438

Note You need to log in before you can comment on or make changes to this bug.