590666 - Firefox path detection fail on Window7 64bit in cfx

Status: VERIFIED FIXED

(Testing :: Mozbase, defect)

Description

[Alexandre Poirot [:ochameau]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; fr; rv:1.9.2.8) Gecko/20100722 Firefox/3.6.8
Build Identifier: 

On Windows 7 and more precisely on 64bit version, there is two "Program Files" directories : 
C:\Program Files (x86)
and
C:\Program Files
Cuddlefish only check the second one and fails to detect Firefox installations.

Reproducible: Always

Steps to Reproduce:
1. Be on Windows7 64bit version
2. Install firefox in default directory
3. launch cfx testall
Actual Results:  
Firefox is not automatically detected, so cfx testall fails.

Expected Results:  
cfx testall pass by detecting firefox path

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Fix this and detect Minefield installation too

Comment 2

[Dietrich Ayala (:dietrich)]

Cool, thanks for the patch Alexandre!

Comment 3

[Atul Varma [:atul]]

Thanks Alexandre! The only thing is, the version of mozrunner we use is really homed here:

  http://github.com/mozautomation/mozmill/tree/1.4.2/mozrunner

And we just copy over that tree to the Jetpack SDK every so often. It seems like the right way to proceed, then, is to re-categorize this bug under Testing->Mozrunner and have harthur review it, which I'll do now.

Comment 4

[Heather Arthur [:harth]]

I think this is bug 554709, which we got a fix for recently, has Jetpack gotten that fix yet?

Comment 5

[Heather Arthur [:harth]]

Comment on attachment 469155 [details] [diff] [review]
Fix this and detect Minefield installation too

I think we might as well look in more places for the binary. This looks good, but change all the 'os.environ["ProgramFiles"]' to 'os.environ.get("ProgramFiles")' so that it doesn't throw a KeyError on non-64 bit Windows, and make the patch from the mozrunner directory if you can, thanks!

Comment 6

[Heather Arthur [:harth]]

I think we want to take this for our next mozrunner release, so I went ahead and pushed it with the get() changes:

http://github.com/mozautomation/mozmill/commit/63a79f611b422cd2893f737261a44e64af2a56c1

Thanks for the patch!

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #6)
> I think we want to take this for our next mozrunner release, so I went ahead
> and pushed it with the get() changes:
> 
> http://github.com/mozautomation/mozmill/commit/63a79f611b422cd2893f737261a44e64af2a56c1

Please don't push it yet. We have to wait until Clint has renamed the branch and tagged the 1.5 release. Pushing patches now will bust our source stamp.

Comment 8

[Heather Arthur [:harth]]

1.5.1:
http://github.com/mozautomation/mozmill/commit/f32170792397dda1cad08cbb0bb54aef9605a59d

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Alexandre, does it now work for you with the latest version of mozrunner from the hotfix-1.5.1 branch? Would be nice if you could mark this bug as verified fixed. Thanks.

Comment 10

[Alexandre Poirot [:ochameau]]

Yes, in this branch, a simple call to mozunner launch a firefox instance from Program Files x86 directory! (win7+64bits)

I'm not used to bugzilla, were you waiting since the commit a VERIFIED from me ?
I mean, do the bug reporter has the responsability to do this ?
Or do you just want me to confirm that all is correctly working now ?
(just to know for the next bug report ;))

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Well, I don't with Jetpack so it's kinda hard for me to verify the fix. For Mozmill 1.5.1 I want to have all the bugs verified as usual. Thanks for looking at it!