Error getting 'Home' directory in PluginProvider.jsm line 185 in msys shell (e.g. running mochitests)

VERIFIED FIXED in mozilla6

Status

()

Toolkit
Add-ons Manager
P3
normal
VERIFIED FIXED
8 years ago
7 years ago

People

(Reporter: rstrong, Assigned: mossop)

Tracking

Trunk
mozilla6
x86
Windows 7
Points:
---
Bug Flags:
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Warning: WARN addons.manager: Exception calling callback: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: file:///d:/moz/_1_mozilla-central/ff-opt/dist/bin/modules/PluginProvider.jsm :: anonymous :: line 185"  data: no]

This currently breaks app update when it tries to check for incompatible add-ons.
blocking2.0: --- → ?
Priority: -- → P1
Severity: normal → blocker
(Assignee)

Comment 2

8 years ago
Strange, doesn't every platform implement "Home"?
Looks like this only happens when running mochitest on Windows so removing the panic flags. Is there a way to get a list of the types so I can then call getAddonsByTypes and exclude plugins in app update?
Severity: blocker → normal
blocking2.0: ? → ---
Priority: P1 → P3
Summary: Error getting 'Home' directory in PluginProvider.jsm line 185 → Error getting 'Home' directory in PluginProvider.jsm line 185 in msys shell (e.g. running mochitests)
(Assignee)

Comment 4

8 years ago
It looks like on Windows getting "Home" should get through to here, are these environment variables missing? http://mxr.mozilla.org/mozilla-central/source/xpcom/io/SpecialSystemDirectory.cpp#717
Right band it is set. msys uses /c/ for the drive which likely confuses the code
s/band/and/

It is usually incorrect to use env vars to get well known paths on Windows... there are API calls to accomplish this. Below are the associated env vars for a cmd.exe and msys shells

cmd.exe
HOMEDRIVE=C:
HOMEPATH=\Users\username
USERPROFILE=C:\Users\username

msys
HOME=/c/Users/username
HOMEDRIVE=C:
HOMEPATH='\'
USERPROFILE=C:\Users\username
From a quick search on mxr it looks like "Home" is only used by the following on Windows (appPicker.js actually uses ProgF on Windows and browser_420786.js returns early except on Linux)
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/filepicker/content/filepicker.js#183
(Assignee)

Comment 8

8 years ago
What I think we want here is the "C:\Documents and Settings\<username>" on XP, "C:\Users\<username>" on Windows 7 and whatever it is on Vista. Home gives us that in some cases but maybe we should make that use an API instead now. I can't see a different directory service definition that covers it.
(Assignee)

Comment 9

8 years ago
In fact it looks like we need to talk to the windows API with CSIDL_PROFILE. That seems to be used in a couple of places for WINCE but not for regular windows.
Duplicate of this bug: 582841
(Assignee)

Comment 11

8 years ago
Created attachment 466698 [details] [diff] [review]
patch rev 1

Spinning this through the try server now but this seems to pass on my machines. It attempts to use CSIDL_PROFILE to get the home directory, if not attempts the previous methods that were there. The test verifies against USERPROFILE which appears to be correct on my windows box and tinderbox.
Assignee: nobody → dtownsend
Status: NEW → ASSIGNED
Attachment #466698 - Flags: review?(jmathies)

Comment 12

8 years ago
Comment on attachment 466698 [details] [diff] [review]
patch rev 1

Technically, I don't think I can approve xpcom/io patches. So you might want a sr. But this looks far more correct to me than what we're currently doing with GetEnvironmentVariableW.
Attachment #466698 - Flags: review?(jmathies) → review+

Updated

8 years ago
Attachment #466698 - Flags: superreview+
(Assignee)

Updated

8 years ago
Whiteboard: [checkin-needed-post-branch]
(Assignee)

Updated

7 years ago
Whiteboard: [checkin-needed-post-branch] → [can land fx6]
(Assignee)

Comment 13

7 years ago
Landed: http://hg.mozilla.org/mozilla-central/rev/bc60f89fe179
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Whiteboard: [can land fx6]
Target Milestone: --- → mozilla6
Verified fixed based on check-in and passing tests.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.