Closed Bug 666717 Opened 11 years ago Closed 11 years ago

URL bar highlighting treats URLs with hyphenated schemes (e.g. view-source) as scheme-less

Categories

(Firefox :: Address Bar, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 7

People

(Reporter: dao, Assigned: dao)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

No description provided.
Severity: enhancement → normal
Attached patch patch (obsolete) — Splinter Review
I took the list of allowed characters from <http://en.wikipedia.org/wiki/URI_scheme#Generic_syntax>.
Attachment #541481 - Flags: review?(gavin.sharp)
Blocks: 665580
Keywords: regression
Attached patch patchSplinter Review
Err, instead of changing tragetValue to targetValue, I did it the other way around...
Attachment #541481 - Attachment is obsolete: true
Attachment #541484 - Flags: review?(gavin.sharp)
Attachment #541481 - Flags: review?(gavin.sharp)
Attachment #541484 - Flags: review?(gavin.sharp) → review+
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).
> 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
http://hg.mozilla.org/mozilla-central/rev/e0b805cab438
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 7
Depends on: 1250662
You need to log in before you can comment on or make changes to this bug.