415367 - ieTab extension not working due to use of ":" in chrome URIs

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[(no longer active)]

Spinoff from bug 415218 comment 16.

The ieTab extension does not work on trunk, because it uses URIs in form of <chrome://ietab/content/reloaded.html?url=http://www.google.com/> and the code highlighted in the URL of this bug blindly rejects the ":" character inside chrome URIs.  This seems to be a regression from bug 413250, which affects both trunk and 1.8.1.12.  So, this may be observable when Firefox 2.0.0.12 is released.  I'm currently downloading 2.0.0.11rc4 to test this.

This should be fixed in Firefox 3 beta 3 as well, so I'm setting the target milestone appropriately to get the appropriate attention.

To test the problem, enter the following code in the error console:

var ios = Components.classes["@mozilla.org/network/io-service;1"].getService(Components.interfaces.nsIIOService); alert(ios.newURI("chrome://blah/blah/?url=%3A", null, null).spec);

The error message observed in the console:

Error: uncaught exception: [Exception... "Access to restricted URI denied"  code: "1012" nsresult: "0x805303f4 (NS_ERROR_DOM_BAD_URI)"  location: "javascript:%20var%20ios%20=%20Components.classes["@mozilla.org/network/io-service;1"].getService(Components.interfaces.nsIIOService);%20alert(ios.newURI("chrome://blah/blah/?url=%253A",%20null,%20null).spec); Line: 1"]

Comment 1

[Philip Chee]

Reversing direction of the dependency (Given that all the patches are in Bug 413250).

Comment 2

[(no longer active)]

It seems like branch already handles this correctly:

<http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/chrome/src/nsChromeRegistry.cpp&rev=1.338.2.8&root=/cvsroot&mark=689-692#668>

I'm preparing a patch for trunk with a unit test.

Comment 3

[(no longer active)]

Attachment: Patch (v1)

First take.

Comment 4

[Brian Polidoro]

This gets fixed by Bug 415338.  Although that doesn't have a test.

Comment 5

[(no longer active)]

Attachment: Patch (v2)

The unit test of the previous patch was mistakenly included in the patch, and was not the unit test that I got to pass.  Here is the updated patch with the correct unit test which passes with the patch applied.

Comment 6

[(no longer active)]

OK, this doesn't affect branch, sorry for the bug spam.  But it still affects trunk and should be fixed for beta 3 because it can break many extensions.

Comment 7

[Daniel Veditz [:dveditz]]

(In reply to comment #1)
> Reversing direction of the dependency (Given that all the patches are in Bug
> 413250).

By convention we make regressions "block" the causing bug, not the other way around. You can argue either way, but sticking with convention makes it less likely regressions will be missed when patches are ported to branches.

Comment 8

[Daniel Veditz [:dveditz]]

Since the patch in bug 415338 got approval I've checked it in.

Comment 9

[(no longer active)]

Attachment: Unit test

The unit test should still be worth considering I guess.

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 301039 [details] [diff] [review]
Unit test

r/sr=dveditz

Comment 11

[(no longer active)]

Comment on attachment 301039 [details] [diff] [review]
Unit test

This unit test should be used to make sure bugs such as this one don't appear in the future.  Seeking approval to land this test.

Comment 12

[Reed Loden [:reed]]

Comment on attachment 301039 [details] [diff] [review]
Unit test

You don't ever need approval for tests.

Comment 13

[Reed Loden [:reed]]

RCS file: /cvsroot/mozilla/chrome/test/unit/test_bug415367.js,v
done
Checking in chrome/test/unit/test_bug415367.js;
/cvsroot/mozilla/chrome/test/unit/test_bug415367.js,v  <--  test_bug415367.js
initial revision: 1.1
done