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

VERIFIED FIXED

Status

()

VERIFIED FIXED
11 years ago
11 years ago

People

(Reporter: ajschult784, Assigned: martijn.martijn)

Tracking

({regression})

Trunk
regression
Points:
---
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [dbaron-1.9:RuCo])

Attachments

(4 attachments)

(Reporter)

Description

11 years ago
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.
(Assignee)

Comment 1

11 years ago
Created attachment 284608 [details]
testcase
(Assignee)

Comment 2

11 years ago
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?
(Assignee)

Comment 3

11 years ago
Or maybe this:
+     <vbox id="info.box" onmousedown="document.commandDispatcher.focusedElement.blur()">
in commonDialog.xul?
Why does this only affect dialogs with text fields (e.g. prompt())?
(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...
Your suggestion should work too, I guess, although it seems a bit hacky...
Flags: blocking1.9?
OS: Linux → All
Hardware: PC → All
Keywords: regression
Duplicate of this bug: 399804
Summary: Textfield does not initially have focus in dialogs → textfield does not initially have focus in dialogs (e.g. prompt())
Flags: blocking1.9? → blocking1.9+
Whiteboard: [dbaron-1.9:RuCo]

Comment 9

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

Comment 10

11 years ago
They have for me, maybe that's some kind of mac weirdness?
(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.
(Assignee)

Comment 12

11 years ago
So on the mac, the "ok" button is meant to get focus? Why is that ifndef then even there?
(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

11 years ago
Perhaps you can alter the tabindex of the description?
(Assignee)

Comment 15

11 years ago
Created attachment 285762 [details] [diff] [review]
my patch
(Assignee)

Comment 16

11 years ago
Created attachment 285764 [details] [diff] [review]
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.
(Assignee)

Comment 17

11 years ago
Created attachment 285766 [details] [diff] [review]
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/.
(Assignee)

Updated

11 years ago
Attachment #285766 - Flags: review?(neil)

Comment 18

11 years ago
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.
Attachment #285766 - Flags: review?(neil) → review+
(Assignee)

Comment 19

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

Ok, thanks for the info, I'll leave xpfe alone then.
Attachment #285766 - Flags: approval1.9?
Duplicate of this bug: 400885
Comment on attachment 285766 [details] [diff] [review]
Neil's suggestion

We should land this for M9, simple fix with very low risk.
Attachment #285766 - Flags: approvalM9?
Comment on attachment 285766 [details] [diff] [review]
Neil's suggestion

aM9=beltzner for endgame drivers
Attachment #285766 - Flags: approvalM9?
Attachment #285766 - Flags: approvalM9+
Attachment #285766 - Flags: approval1.9?
Attachment #285766 - Flags: approval1.9+
Assignee: nobody → martijn.martijn
(Assignee)

Comment 23

11 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Duplicate of this bug: 399167
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.