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

RESOLVED FIXED in Firefox 7

Status

()

Firefox
Location Bar
RESOLVED FIXED
6 years ago
a year ago

People

(Reporter: dao, Assigned: dao)

Tracking

(Depends on: 1 bug, {regression})

Trunk
Firefox 7
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Updated

6 years ago
Severity: enhancement → normal
(Assignee)

Comment 1

6 years ago
Created attachment 541481 [details] [diff] [review]
patch

I took the list of allowed characters from <http://en.wikipedia.org/wiki/URI_scheme#Generic_syntax>.
Attachment #541481 - Flags: review?(gavin.sharp)
(Assignee)

Updated

6 years ago
Blocks: 665580
Keywords: regression
(Assignee)

Comment 2

6 years ago
Created attachment 541484 [details] [diff] [review]
patch

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+

Comment 3

6 years ago
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).
(Assignee)

Comment 4

6 years ago
> 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
(Assignee)

Comment 5

6 years ago
http://hg.mozilla.org/mozilla-central/rev/e0b805cab438
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 7
Depends on: 668753

Updated

a year ago
Depends on: 1250662
You need to log in before you can comment on or make changes to this bug.