Closed
Bug 323455
Opened 18 years ago
Closed 18 years ago
chrome override URIs not resolved
Categories
(Toolkit :: Startup and Profile System, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: chpe, Assigned: chpe)
Details
(Keywords: fixed1.8.1.5, Whiteboard: [need testcase])
Attachments
(1 file)
1.23 KB,
patch
|
benjamin
:
first-review+
dveditz
:
approval1.8.1.5+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•18 years ago
|
||
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)
Comment 2•18 years ago
|
||
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.
Updated•18 years ago
|
Attachment #208495 -
Flags: first-review?(darin) → first-review+
Updated•18 years ago
|
Assignee: nobody → chpe
Assignee | ||
Comment 3•18 years ago
|
||
Do I need to request a second-review for the patch, or is first-review enough for checkin?
Comment 4•18 years ago
|
||
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?
Assignee | ||
Comment 5•18 years ago
|
||
The patch still applies cleanly: patching file chrome/src/nsChromeRegistry.cpp Hunk #1 succeeded at 2416 (offset 32 lines).
Updated•18 years ago
|
Whiteboard: [checkin needed]
Comment 6•18 years ago
|
||
chrome/src/nsChromeRegistry.cpp 1.349
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Comment 7•17 years ago
|
||
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 8•17 years ago
|
||
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+
Updated•17 years ago
|
Whiteboard: [checkin needed (1.8branch)]
Comment 9•17 years ago
|
||
chrome/src/nsChromeRegistry.cpp 1.338.2.5
Keywords: fixed1.8.1.5
Whiteboard: [checkin needed (1.8branch)]
Comment 10•17 years ago
|
||
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]
Assignee | ||
Comment 11•17 years ago
|
||
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.
Description
•