Closed
Bug 555715
Opened 15 years ago
Closed 15 years ago
Replace resource://app/ with resource:///
Categories
(MailNews Core :: Build Config, defect)
MailNews Core
Build Config
Tracking
(Not tracked)
VERIFIED
FIXED
Thunderbird 3.1b2
People
(Reporter: kairo, Assigned: kairo)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
http://mxr.mozilla.org/comm-central/source/mozilla/netwerk/protocol/res/src/nsResProtocolHandler.cpp#178 says that resource:/// points to the application directory and below it resource://gre/ is defined to point to the GRE directory, but I can't find any documentation that might point out that resource://app/ even exists.
This might be the reason why we see errors with those URLs in debug builds, e.g. when running xpcshell tests.
We should replace resource://app/ with resource:/// where we have it in our code.
Assignee | ||
Comment 1•15 years ago
|
||
OK, I can confirm that doing that replacement makes the errors in xpcshell tests go away. Right now, we're throwing NS_ERROR_NOT_AVAILABLE for every Components.utils.import line using a resource://app/ URI, with a patch replacing resource://app/ with resource:/// we are not throwing any more.
Component: UI Design → Build Config
Product: SeaMonkey → MailNews Core
QA Contact: ui-design → build-config
Assignee | ||
Comment 2•15 years ago
|
||
Working on a patch, not sure which component it belongs in, as it touches all of mailnews, mail, and suite.
Assignee: nobody → kairo
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•15 years ago
|
||
This patch is a simple find-and-replace pass over all of mail, mailnews, and suite. This apparently invalid URI construct is not found anywhere outside those directories.
Attachment #435637 -
Flags: review?(bugzilla)
Comment 4•15 years ago
|
||
Where are the errors happening?
I apparently explicitly added a case to mailDirService.js to deal with this in mailnews tests:
http://mxr.mozilla.org/comm-central/source/mailnews/test/resources/mailDirService.js#64
Comment 5•15 years ago
|
||
The resource "resource://app/" makes sense to me, semantically.
Why not just add a resource that makes it valid (and points to the same dir as resource:///) instead of the global search/replace?
Comment 6•15 years ago
|
||
Heh, I was wondering where we got to use the idea "app" from, and a gloda search found this:
https://bugzilla.mozilla.org/show_bug.cgi?id=463278#c9
Not sure it is the absolute genesis.
Assignee | ||
Comment 7•15 years ago
|
||
(In reply to comment #4)
> Where are the errors happening?
>
> I apparently explicitly added a case to mailDirService.js to deal with this in
> mailnews tests:
> http://mxr.mozilla.org/comm-central/source/mailnews/test/resources/mailDirService.js#64
Interesting. I saw it happening in non-mailnews tests as SeaMonkey loads nsSuiteGlue.js (which loads migration.jsm from mailnews) and msgAsyncPrompter.js on every startup, and those failed to load with that throw.
I agree that resource://app/ would be somewhat clearer in meaning than resource:/// but we'd need to add it to the global version, IMHO, as else we're basically loading invalid URIs (even though they seem to load OK in most cases).
And in the end, resource:/// means exactly what we mean with resource://app/.
Assignee | ||
Comment 8•15 years ago
|
||
(In reply to comment #6)
> Heh, I was wondering where we got to use the idea "app" from, and a gloda
> search found this:
>
> https://bugzilla.mozilla.org/show_bug.cgi?id=463278#c9
>
> Not sure it is the absolute genesis.
Hrm. OK, I take the blame, I just was wrong but everybody accepted what I said. :(
Comment 9•15 years ago
|
||
Thunderbird might actually be establishing the "app" mapping at some point, but I think the main issue is that, as KaiRo says, we would realy need to establish it at the dawn of time with the other mappings for perfect consistency.
This very likely explains why all my resource://app/ imports from inside JS XPCOM components didn't work during the top-level evaluation. (I think I eventually cottoned onto it, but assumed it was an intentional platform limitation...)
Thanks for tracking this down and preparing a patch!
Comment 10•15 years ago
|
||
This bitrotted already :-( Still I've fixed that and run the Thunderbird tests against it and it all seems to work fine :-) r=Standard8
Attachment #435637 -
Attachment is obsolete: true
Attachment #436537 -
Flags: review+
Attachment #435637 -
Flags: review?(bugzilla)
Assignee | ||
Comment 11•15 years ago
|
||
I actually had updated the patch by myself for bitrot and new additions, Standard8 agreed over IRC I can take his review for that, so I pushed it as http://hg.mozilla.org/comm-central/rev/ffe61760113a
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.1b2
Version: unspecified → Trunk
Updated•15 years ago
|
Flags: in-testsuite-
Updated•10 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•