Closed Bug 613763 Opened 14 years ago Closed 14 years ago

Form elements can regain focus under a tab-modal prompt

Categories

(Toolkit :: General, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b8
Tracking Status
blocking2.0 --- final+

People

(Reporter: void.at, Assigned: Dolske)

References

Details

Attachments

(3 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:2.0b8pre) Gecko/20101120 Firefox/4.0b8pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:2.0b8pre) Gecko/20101120 Firefox/4.0b8pre

*Filed a new bug as requested (blocking bug 59314)*

I've just tried the patch for Fx 4.0 b8 and looks great, but I think there are a couple of small accessibility issues:

1. Form elements can still gain focus and react to input.


2. Prompt doesn't close on Enter --this may have been mentioned before.


3. Blurring the background may not be the best option, since sometimes the decision of what to do depends on what's on the screen at that moment.

IMO, darkening the BG *a little* --changing rgba(247,247,247,.15) for rgba(0,0,0,.15)-- plus the radial effect it already has should be enough to direct users to the dialog "windows".


4. With common dialogs, it often happens that users end up choosing an option they didn't even see because they were already typing when it showed up, so the dialog reacts to an input it wasn't meant for it. Is it possible to add a small delay (like 0.5s) where all input in temporarily ignored?.

Reproducible: Always
Blocks: 59314
Attached file Test case
Click on any button to show a dialog and use the tab-key to loop through the fields (without closing the dialog).
Thanks for pointing the dups.

On an additional note on point 4... if it can be done, it would be better if it was configurable - delays seem to bother people often, because they're seen as slowness.
I'm going to mutate this bug a bug to just deal with issue #1. Multiple issues per-bug are hard to work with, especially when they're likely caused by different things.

Issues #2 and 3 already have bugs on file (see comment 2).

Issue #4 is interesting -- can you file a new bug just for that? We've talked about doing this kind of thing elsewhere too.

STR for form bug:

1) open google.com
2) enter javascript:alert("test") in the URL bar (tab-modal prompt displayed)
3) Switch to another tab
4) Switch back to tab from step 1.
5) Notice a (blurred) cursor is flashing in the page's search box. Typing text and pressing return performs a search but the prompt stays. [Actually, this is a bit of a weird case, because Google Instant isn't actually loading a new page. If you turn off Google instant, a search is performed and the prompt goes away when the page navigates.]

In any case, the form field shouldn't be refocused, and I don't think the navigation is supposed to work even it it is.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Product: Firefox → Toolkit
QA Contact: general → general
Summary: Usability issues of patch for Bug 59314 → Form elements can regain focus under a tab-modal prompt
blocking2.0: --- → final+
Hmm, so I think this is being caused by tabbrowser.xml :: updateCurrentBrowser().

The last line there is calling the focus mananger to set focus back to the newly-selected <browser> for the tab. If I comment it out the form isn't getting focused. I think we want to suppress this when there's a tab-modal prompt. Or even focus the prompt itself, if that's needed.

I wonder if the prompt, upon dismissal, may need to explicitly refocus the <browser> is some cases.
Attached patch Patch v.1 (obsolete) — Splinter Review
2 birds, 1 patch... Also fixes the focus problem described in bug 614379.

The "-moz-user-focus: none" on the <browser> prevents tabbing into it (as 614379 describes), and also seems to make the tabbrowser code mentioned in comment 5 do nothing.

The focus manager bit added to onPromptClose ensures that focus returns to the previous spot in the page. I'm least about this part -- not sure if I need to do more to consider weird edge cases (eg frames), or how to detect/handle a prompt in a background tab being closed (not sure how a user would trigger that!). Maybe this is good enough for the immediate problem, and any weird bits can be fixed in a followup?

I'm also wary of this being a simple 8-line patch to fix a focus problem, guess that's just a testament to all the focus work Enn did earlier. :D
Assignee: nobody → dolske
Attachment #492901 - Flags: review?
Comment on attachment 492901 [details] [diff] [review]
Patch v.1

sigh, reviewflag fail.
Attachment #492901 - Flags: review? → review?(enndeakin)
Comment on attachment 492901 [details] [diff] [review]
Patch v.1

> I'm least about this part -- not sure if I need to do more to
> consider weird edge cases (eg frames)

Frames should work as long as 'domWin' is a top-most window, that is, the one directly inside the <browser>. Is that always the case?

The one thing not handled though is the F6 key to cycle between frames, which still allows the document to be focused. You could add a listener to trap that and, if the target is the blocked document, shift focus to the prompt's ok button.

> or how to detect/handle a prompt in a background tab being closed

fm.setFocus handles non-visible windows so you should be ok there.


What happens if a script tries to focus something in a window that is showing a prompt? Was that blocked before?
Attachment #492901 - Flags: review?(enndeakin) → review+
(In reply to comment #8)

> Frames should work as long as 'domWin' is a top-most window, that is, the one
> directly inside the <browser>. Is that always the case?

No, domWin here is the window which had alert/prompt/confirm called on it.

I guess this should just be domWin.top here. Which makes sense now that I ponder it... If you have Page A containing Frames X and Y, the prompt blocks A+X+Y, and so when the prompt goes away focus should be restored to whereever it was, be it in A or X or Y.

> The one thing not handled though is the F6 key to cycle between frames, which
> still allows the document to be focused. You could add a listener to trap that
> and, if the target is the blocked document, shift focus to the prompt's ok
> button.

Hmm, I had no idea about F6. Will look into this.

> What happens if a script tries to focus something in a window that is showing a
> prompt? Was that blocked before?

Content script shouldn't be running. Not sure what happens if chrome/addon code does this (or what happened before), will also look at this.
(In reply to comment #10)
> Content script shouldn't be running.

It could be running in another window.
(In reply to comment #11)
> (In reply to comment #10)
> > Content script shouldn't be running.
> 
> It could be running in another window.

Looks like we already block this... With this test case I see the tab-modal prompt come up in the new window, and while it's up the input's value changes but focus doesn't move.
Attached patch Patch v.2Splinter Review
I wasn't able to get the F6 listener working. It can't just be a <handler> in tabprompts.xml, because it needs to run when key input is targeting other parts of the window (like the URL bar). Even then, as a global listener, it would need to know where focus is going to shift _to_, not what the key event's target is.

At least, AIUI, the relevant code is nsEventStateManager::PostHandleEvent(), wherein the keypress==F6 case is falling focus manager's moveFocus with MOVEFOCUS_FORWARDDOC / MOVEFOCUS_BACKWARDDOC.

I suppose the alternative would be for the tabprompt to add a capturing focus listener on domWin.top, and cancel/blur it?

But shouldn't the focus manager be skipping over the page when it's inside a non-focusable browser / parent chain? Seems like this should be suppressed just as tab is.
Attachment #492901 - Attachment is obsolete: true
Blocks: 617872
Pushed http://hg.mozilla.org/mozilla-central/rev/695ee208215d

Filed bug 617872 to address the F6 issue.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
I've noticed that we can still search in the documents (quick search), and I must say *I don't think this is an error*, again, thinking that we sometimes need to see what's in the document to actually make a decision.

Now, following this same logic, I think disabling the scroll-bars _could_ make it look like an inconsistency. Any thoughts?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: