285416 - Access to Firefox Options from Start Menu broken

Status: RESOLVED FIXED

(Firefox :: Shell Integration, defect, P1)

Description

[Ali Ebrahim]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050308
Firefox/1.0+

If Firefox is your default browser on Windows XP, it will show in the top left
of the Start Menu. You can right click on it, and find an option called 'Firefox
Options'. Clicking this used to bring up the Firefox options, but now brings a
tiny window without a title (see forthcoming screenshot). This is probably a
regression from the Prefwindow V checkin.

Steps to repro:

1. Have Firefox be your default browser
2. Click on Start Menu
3. Right click on Firefox in top left of Start Menu
4. Choose 'Firefox Options'

Expected:

Brings up Firefox options

Actual:

Tiny nameless window pops up

Comment 1

[Ali Ebrahim]

Attachment: Screenshot of tiny window described in comment 0

Comment 2

[:Gavin Sharp [email: [e-mail]]]

I have a patch for this coming up.

Comment 3

[Mike Connor [:mconnor]]

Requirements for this patch:

Has to obviously fix the shellservice to set the new URL.

Has to migrate obsolete registry settings during default browser checking,
without  user interaction if its the only change.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #3)
> Requirements for this patch:
> Has to obviously fix the shellservice to set the new URL.

I thought it would be better to introduce a "-preferences" parameter, to
eliminate the need to hardcode the value and prevent us from having to do such
cleanup in the future.

> Has to migrate obsolete registry settings during default browser checking,
> without user interaction if its the only change.

Is default browser checking done at each startup regardless of the "ask me..."
pref? This is the part I haven't gotten to yet, I should be able to look into it
tonight.

Comment 5

[Mike Connor [:mconnor]]

Good point, but if you add a command-line handler, it'll have to work on all
platforms, and I'm not sure how well that'd work on Linux with an open app.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Attachment: Patch v1 - add the command line handler

This adds a "-preferences" handler to remove the need to hardcode the chrome
URL in the registry, and makes the appropriate change to the default browser
code. It also makes a change to another caller that has the old pref URL
hardcoded.

This doesn't fix cases where the old registry key is already set, which would
be most existing installs I assume.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Attachment: Patch v2

This patch:
> Adds the -preferences command line param so that it's not necessary to hard
code the chrome URL
> Checks for the old pref URL and handles it separately

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Attachment: Patch v3

The last patch had some problems with focusing the dialog when a window was
already open. I'll do some more testing before asking for review, but I think
that this should work in all cases.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Attachment: Patch v4

This has been tested on Windows and works fine in all situations I can think of
(options window already opened, browser already opened, no browser opened, etc)
using both "-preferences" and "-chrome chrome://browser/content/pref/pref.xul"
(the old pref URL). It shouldn't be any different on Mac/Unix, but I haven't
tested there.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Attachment: Patch v4 (corrected)

This has been tested on Windows and works fine in all situations I can think of
(options window already opened, browser already opened, no browser opened, etc)
using both "-preferences" and "-chrome chrome://browser/content/pref/pref.xul"
(the old pref URL). It shouldn't be any different on Mac/Unix, but I haven't
tested there.

Comment 11

[Mike Connor [:mconnor]]

Comment on attachment 180861 [details] [diff] [review]
Patch v4 (corrected)

r=me

requesting approval (rather late) this should be fairly safe, and fixes
something that'll be obviously broken otherwise.

Comment 12

[Mike Connor [:mconnor]]

Comment on attachment 180861 [details] [diff] [review]
Patch v4 (corrected)

bloody hell.

Comment 13

[Asa Dotzler [:asa]]

Comment on attachment 180861 [details] [diff] [review]
Patch v4 (corrected)

a=asa

Comment 14

[:Gavin Sharp [email: [e-mail]]]

Checked in by mconnor.