399553 - textfield does not initially have focus in dialogs (e.g. prompt())

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Andrew Schultz]

With current trunk (after bug 60513 landed), dialogs with a textfield (like the master password dialog) seem to have the text focused rather than textfield.  Tabbing into the textfield works fine, but I'd expect to just be able to start typing when the dialog comes up.

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

Comment 2

[Martijn Wargers (dead)]

Apparently, the dialog binding is focusing the first focusable element in the dialog, see bug 322155.

I need the #info.box element to be focusable, otherwise it's not possible to copy text, when the textbox is focused.

I'm not sure what's the best way to fix this.
Should I add the -moz-user-focus after a while or something like that?

Comment 3

[Martijn Wargers (dead)]

Or maybe this:
+     <vbox id="info.box" onmousedown="document.commandDispatcher.focusedElement.blur()">
in commonDialog.xul?

Comment 4

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

Why does this only affect dialogs with text fields (e.g. prompt())?

Comment 5

[Martijn Wargers (dead)]

I think because of this code here:
http://lxr.mozilla.org/seamonkey/source/toolkit/content/commonDialog.js#170

Comment 6

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

(In reply to comment #5)
> I think because of this code here:
> http://lxr.mozilla.org/seamonkey/source/toolkit/content/commonDialog.js#170

Hrm, I see. That would prevent this bug from affecting dialogs without textfields, yeah. The code in dialog.xml (focusInit) that was added in bug 322155 does basically the same thing, but it only does it if there are no focusable elements other than buttons, rather than if there are no text fields.

I suppose we could add an "else" there and call document.commandDispatcher.advanceFocus(); to get focus past the description, or just focus the textbox explicitly...

Comment 7

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

Your suggestion should work too, I guess, although it seems a bit hacky...

Comment 9

[Jesse Ruderman]

alert() dialogs don't have focus on the OK button.  Is that the same bug?

Comment 10

[Martijn Wargers (dead)]

They have for me, maybe that's some kind of mac weirdness?

Comment 11

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

(In reply to comment #9)
> alert() dialogs don't have focus on the OK button.  Is that the same bug?

Yeah, it is. The ifndef http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/content/commonDialog.js&rev=1.13#170 makes this apply to all commonDialogs on Mac.

Comment 12

[Martijn Wargers (dead)]

So on the mac, the "ok" button is meant to get focus? Why is that ifndef then even there?

Comment 13

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

(In reply to comment #12)
> So on the mac, the "ok" button is meant to get focus? Why is that ifndef then
> even there?

Because it interferes with the Mac default button handling in dialog.xml.

Comment 14

[[e-mail]]

Perhaps you can alter the tabindex of the description?

Comment 15

[Martijn Wargers (dead)]

Attachment: my patch

Comment 16

[Martijn Wargers (dead)]

Attachment: gavin's idea from comment 6

I tried to use document.commandDispatcher.advanceFocus() but for some reason, I got a js error with that (and failed to work).

The problem with this patch is, that the content of the textbox isn't selected, like it should. Also, I guess this still might keep the problem on the Mac, if I understand correctly.

Comment 17

[Martijn Wargers (dead)]

Attachment: Neil's suggestion

Thanks Neil, you're suggestion seems the ideal one. Setting the description's tabindex to -1 makes it focusable by clicking on the mouse, but not tabbable.

I'm a bit confused by the xpfe/ directory however.
It seems like commonDialog.js is not used in the xpfe/ directory, but rather the one from the toolkit/ directory.
If the one from the xpfe/ directory would be used, I also would need to change stuff in commonDialog.js from xpfe/.

Comment 18

[[e-mail]]

Comment on attachment 285766 [details] [diff] [review]
Neil's suggestion

Don't patch xpfe chrome, it's dying.

I was worried about keyboard accessibility, but I was pleased to discover that if one of the buttons has focus then you can use Ctrl+A and Ctrl+C to select and copy the text.

Comment 19

[Martijn Wargers (dead)]

Comment on attachment 285766 [details] [diff] [review]
Neil's suggestion

Ok, thanks for the info, I'll leave xpfe alone then.

Comment 21

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

Comment on attachment 285766 [details] [diff] [review]
Neil's suggestion

We should land this for M9, simple fix with very low risk.

Comment 22

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

Comment on attachment 285766 [details] [diff] [review]
Neil's suggestion

aM9=beltzner for endgame drivers

Comment 23

[Martijn Wargers (dead)]

Checking in commonDialog.xul;
/cvsroot/mozilla/toolkit/content/commonDialog.xul,v  <--  commonDialog.xul
new revision: 1.14; previous revision: 1.13
done

Checked into trunk.

Comment 25

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

Verfied fixed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007102504 Minefield/3.0a9pre

Within the master password dialog the textfield gets the focus now.