311288 - Browser restart without due warning if "Software Update" dialog appears while typing

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect, P2)

Description

[Thomas Rutter]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20051003 Firefox/1.4.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20051003 Firefox/1.4.1

If the "Software Update" dialog box (which indicates that a new software update
is ready to install) appears while I am typing (say, into a textarea on a Web
page), it immediately steals keyboard focus, and causes Firefox to restart upon
the next press of the space bar.

This is because the button labelled "Restart Firefox Now" is pre-selected as the
default option, and pressing the space bar counts as pressing the currently
selected button as soon as the dialog appears.

Reproducible: Always

Steps to Reproduce:
1. Install branch build 20051002
2. Make sure "Automatically download and install the update" is turned on in prefs.
3. Start typing in textarea
4. Continue until software update for 20051003 is downloaded

Actual Results:  
Software update window appears with "Restart Firefox Now" pre-selected; next
press of space bar causes Firefox to immediately restart, losing browser session.

Expected Results:  
Pressing space bar immediately after the dialog appears should not cause any
action in the dialog - a space bar pressed immediately after a dialog opens
ought to be ignored, as it could be part of something the user was typing before
the dialog appeared.

Three possible solutions.  (1) A one-second timeout after the dialog appears,
before the space bar becomes active.  Or: (2) The dialog does not steal keyboard
focus (after all, it is not modal).  Or: (3) The "Restart Firefox Now" button is
not pre-selected when the dialog appears.

Data loss issue.  I lost my browsing session and what I had been typing into the
form when it happened.

Related to, but more serious than, bug #305383.

Comment 1

[Darin Fisher]

Oops.  Yeah, I think we should fix this for Firefox 1.5 RC1.

Comment 2

[Peter van der Woude [:Peter6]]

*** Bug 311289 has been marked as a duplicate of this bug. ***

Comment 3

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: patch

The simplest fix - focus the "Later" button by default.

Comment 4

[Darin Fisher]

Comment on attachment 199471 [details] [diff] [review]
patch

r=darin

Comment 5

[Asaf Romano (gone)]

Comment on attachment 199471 [details] [diff] [review]
patch

You ought to use the defaultButton property instead; otherwise, this will be
broken on mac (at the very least)

Comment 6

[Asaf Romano (gone)]

Sigh, i forgot we had only implemented the defaultButton property in the dialog
binding. That said, this is still an issue on OS X, where enter *always* fires
|advance()| (which is the Finish button at this point) regardless of the
focused/default button.

Comment 7

[Asa Dotzler [:asa]]

So do we need a new patch here? Asaf, can you help us out?

Comment 8

[Asaf Romano (gone)]

(In reply to comment #7)
> So do we need a new patch here? Asaf, can you help us out?

Three options here:
 * Add some hacks (listening to keypress events) in the update code to make the
Later button behave like a default-button.
 * Implement a xpinstall-like timeout for the Restart button (only in the case
where the window wasn't focused when the wizard advanced to the last page). This
is safe and prolly the way to go; it has a little l10n impact (one string) though.
 * Just focus the later button and realize it's a very partial fix - it will be
broken on OS X (esp. if "full keyboard access" is off); tab->enter and
shift-tab->enter will remain an issue XP.

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

My preferred solution would be to ensure that the user sees the dialog. While
making "Later" the default button would prevent dataloss (and thus be an
acceptable immediate solution to this blocker, IMO) it would still result in a
dialog flashing by the user's face which can often lead to undue stress.

Of the solutions Mano mentions, the timeout-delay on the button seems best
all-around. A short delay (1s) should be enough to catch the keystrokes without
making it feel too awkward, and not require any label changes.

Comment 10

[Asaf Romano (gone)]

It turns out that two seconds are be the very minimal delay to make this
functional, makes me wonder if we have to do update the button label.

Comment 11

[Asaf Romano (gone)]

Attachment: The delay option (two seconds)

see last two comments.

Comment 12

[Asaf Romano (gone)]

Checking in updates.js;
/cvsroot/mozilla/toolkit/mozapps/update/content/updates.js,v  <--  updates.js
new revision: 1.53; previous revision: 1.52
done

Comment 13

[Asaf Romano (gone)]

1.8 branch:
Checking in updates.js;
/cvsroot/mozilla/toolkit/mozapps/update/content/updates.js,v  <--  updates.js
new revision: 1.45.2.6; previous revision: 1.45.2.5
done

Comment 14

[Thomas Rutter]

Please note, that now if you select "Apply Downloaded Update Now" from the Help menu, it isn't possible to press "Restart Firefox Now" for two seconds.

I think the two second rule should only apply when the window appears involuntarily.

Comment 15

[Adam Guthrie]

*** Bug 346030 has been marked as a duplicate of this bug. ***