Closed Bug 555715 Opened 11 years ago Closed 11 years ago

Replace resource://app/ with resource:///

Categories

(MailNews Core :: Build Config, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
Thunderbird 3.1b2

People

(Reporter: kairo, Assigned: kairo)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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
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
Attached patch simple find and replace (obsolete) — Splinter Review
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)
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
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?
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.
(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/.
(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. :(
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!
Attached patch Fixed for bitrotSplinter Review
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)
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: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.1b2
Version: unspecified → Trunk
V.Fixed, per bug 556663 comment 0.
Status: RESOLVED → VERIFIED
Flags: in-testsuite-
See Also: → 1134234
You need to log in before you can comment on or make changes to this bug.