Closed Bug 323455 Opened 18 years ago Closed 18 years ago

chrome override URIs not resolved

Categories

(Toolkit :: Startup and Profile System, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: chpe, Assigned: chpe)

Details

(Keywords: fixed1.8.1.5, Whiteboard: [need testcase])

Attachments

(1 file)

Following the chrome override example on http://developer.mozilla.org/en/docs/Chrome_Registration :

override chrome://global/content/netError.xhtml jar:embedder.jar!/global/content/netError.xhtml

it looks like one should be able to specify a .jar file that resides in the same directory as the .manifest file and have that work without specifying an absolute path to the jar file; but it does not work as expected, since the override URI isn't resolved against the manifest's URI.

Simple patch follows.
Attached patch fixSplinter Review
I'm not sure how review works in this module; I've picked you from looking at the cvs log of recent checkins to nsChromeRegistry.cpp.
Attachment #208495 - Flags: first-review?(darin)
Hrm, I didn't really intend for it to be used this way (I typically meant it to be used as

override chrome://foo/content/url chrome://extension/content/override-file

But I guess this usecase makes some sense.
Attachment #208495 - Flags: first-review?(darin) → first-review+
Assignee: nobody → chpe
Do I need to request a second-review for the patch, or is first-review enough for checkin?
No, per http://www.mozilla.org/projects/toolkit/review.html you only need second-review when your first-reviewer thinks you do, and asks for it.

So, how bit-rotten have you gotten over the course of the last year?
The patch still applies cleanly:
patching file chrome/src/nsChromeRegistry.cpp
Hunk #1 succeeded at 2416 (offset 32 lines).
Whiteboard: [checkin needed]
chrome/src/nsChromeRegistry.cpp 1.349
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Comment on attachment 208495 [details] [diff] [review]
fix

As far as I can tell, this is pretty low-risk, and it'd probably help extension developers to have this actually work on branch also.
Attachment #208495 - Flags: approval1.8.1.5?
Comment on attachment 208495 [details] [diff] [review]
fix

approved for 1.8.1.5, a=dveditz for release-drivers
Attachment #208495 - Flags: approval1.8.1.5? → approval1.8.1.5+
Whiteboard: [checkin needed (1.8branch)]
chrome/src/nsChromeRegistry.cpp 1.338.2.5
Keywords: fixed1.8.1.5
Whiteboard: [checkin needed (1.8branch)]
Christian:  Any chance you can quickly verify this works in the latest 1.8 (2.0.0.5pre) builds and replace the "fixed1.8.1.5" keyword with "verified1.8.1.5"?

Does anyone have a test case/extension that QA can use to verify this?
Whiteboard: [need testcase]
I ran into this problem in my embedding app (Epiphany), not in an extension. I don't have a 1.8 branch build, and the nightlies don't have the necessary stuff to build my app with it.

A testcase could be constructed with a file "myabout.xhtml" and the .manifest file in the same directory, containing:

override chrome://global/content/about.xhtml file:myabout.xhtml

and then visit the "about:" URL. Without the patch, you'll get a file-not-found error; with the patch you get shown your myabout.xhtml file.
Component: XRE Startup → Startup and Profile System
QA Contact: xre.startup → startup
You need to log in before you can comment on or make changes to this bug.