620145 - Prompt for gmail message with no text has to be focused (clicked on) before ok-ing (or canceling)

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows NT 5.1; rv:2.0b9pre) Gecko/20101218 Firefox/4.0b9pre ID:20101218030347

repro:
open gmail
make a new message with title but without text
hit [send]
a modal message pops up asking me if i want send the message w.o text
click [ok]

result:
nothing happens
you havce to click [ok] twice to send the message

this regressed when the dialog changed from global to modal

Comment 1

[Raul Nicolae Malea]

Confirm this bug for Windows 7 x86. Mozilla/5.0 (Windows NT 6.1; rv:2.0b9pre) Gecko/20101218 Firefox/4.0b9pre ID:20101218030347

Comment 2

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

blocking+ for investigation.

Comment 3

[Alice0775 White]

And it need click twice for switching tab/activate menu/execution toolbar button,.
No tooltip or wrong tooltip pops up for toolbar button.

Comment 4

[Ed Morley [:emorley]]

Confirmed using Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20101227 Firefox/4.0b9pre ID:20101227030354

Regression range...

Last good nightly: 2010-11-19 
First bad nightly: 2010-11-20

Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6478c1b83d60&tochange=baa6cc2f72e4

In this range is:
Bug 59314 - Alerts should be content-modal, not window-modal
...so modal alerts likely cause, as expected.

In addition, when the new style alert appears after following the STR, the question mark image does not appear straight away and the tooltip for the OK button is empty and appears as just a grey box. Is this related or should it be logged separately?

Comment 5

[Peter van der Woude [:Peter6]]

(In reply to comment #4)

> In addition, when the new style alert appears after following the STR, the
> question mark image does not appear straight away and the tooltip for the OK
> button is empty and appears as just a grey box. Is this related or should it be
> logged separately?

not the same, file a new bug.
I can confirm, the delay only happens once (the first time).

Comment 6

[Nochum Sossonko [:Natch]]

If you leave the prompt open without dismissing it a stop script dialog comes up for the following line:

file:///C:/Users/Natch/Documents/mozilla_trunk/m-c_release/dist/bin/components/nsPrompter.js:463

Where the prompter is looping over args.promptActive and spinning the event loop (I think). There's something wrong there apparently...

Comment 7

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

Nochum: please file a new bug?

Comment 8

[Nochum Sossonko [:Natch]]

Filed bug 621764, although I think that is the cause for this bug. I've set the dependency as such.

Comment 9

[Nochum Sossonko [:Natch]]

I realize the bug report is a little misleading, so updated the subject accordingly. What's actually happening is that the first click doesn't get caught by the prompt, it just "focuses" it.

Comment 10

[Nochum Sossonko [:Natch]]

A simple and obvious way to repro is:

1) Open two windows
2) In one window, enter data:text/html,<script>confirm("confrim");</script>
3) When the confirm comes up, close the second window with the windows X button

ER: Second window closes and prompt can be dismissed in first window on first click.

AR: Second window doesn't close and prompt needs two clicks to close (after which the second window can be closed).

Comment 11

[Ed Morley [:emorley]]

(In reply to comment #5)
> (In reply to comment #4)
> > In addition, when the new style alert appears after following the STR, the
> > question mark image does not appear straight away and the tooltip for the OK
> > button is empty and appears as just a grey box. Is this related or should it be
> > logged separately?
> 
> not the same, file a new bug.
> I can confirm, the delay only happens once (the first time).
Filed as bug 621672 and bug 622340.

Comment 12

[Justin Dolske [:Dolske]]

Seems focus is shifting back to to the gmail page; but it's weird... Following the STR in comment 0, once the prompt is up:

* Tabbing works as expected, moving around chrome and (apparently) not in content. "Ok" button is focused initially, and pressing tab once moves it to cancel.

* After clicking one of the tab-modal prompt buttons (which should trigger the button's command, but apparently doesn't), focus is shifting into content. Pressing Tab now shifts focus around in the Gmail page! O_o Not sure why focus manager is allowing tab to move around in a page that shouldn't be focusable (maybe it's not expecting focus to be able to get there in the first place?). CCing Enn, note that I was testing with bug 617872's patch applied.


I flipped on the focus manager debugging, starts with me typing in the Subject field:

**Element (none) has been blurred
**Element input has been focused from html [Newdoc: 1 FocusChanged: 1 Raised: 0 Flags: 1002]
Update Caret: 0 1
<<SetFocus>>
Shift Focus: div Flags: 0 Current Window: 0F3FE6D8 New Window: 0F3FE6D8 Current Element: 125797F8 In Active Window: 1 In Focused Window: 1
**Element input has been blurred
Update Caret: 0 1
**Element div has been focused from html [Newdoc: 0 FocusChanged: 1 Raised: 0 Flags: 0]
Update Caret: 1 0
<<SetFocus>>
Shift Focus: button Flags: 0 Current Window: 0F3FE6D8 New Window: 0496A108 Current Element: 0A236188 In Active Window: 1 In Focused Window: 0
**Element div has been blurred
**Element button has been focused from window [Newdoc: 1 FocusChanged: 1 Raised: 0 Flags: 0]
Update Caret: 1 1

*click button*

<<ClearFocus begin>>
<<ClearFocus end>>
<<SetFocusedWindow begin>>
Shift Focus: (none) Flags: 0 Current Window: 0496A108 New Window: 0F3FE6D8 Current Element: 0F0A62C8 In Active Window: 1 In Focused Window: 0
**Element button has been blurred
**Element (none) has been focused from html [Newdoc: 1 FocusChanged: 0 Raised: 0 Flags: 0]
Update Caret: 0 1
<<SetFocusedWindow end>>

Tomorrow I'll try breakpointing to see what's causing the "Element (none) has been focused from html".

Comment 13

[Justin Dolske [:Dolske]]

(oops, forgot to actually CC Enn)

Comment 14

[Neil Deakin]

The first mouse event seems to be fired at the page rather than at the chrome window. The second mouse event is fired at the chrome window.

Comment 15

[Neil Deakin]

This issue relates to the active EventStateManager tracking added by bug 582771 and 603550. Backing out bug 603550, for instance, fixes this bug. Also, activating the Send button with the keyboard works.

Pressing the button calls nsEventStateManager::SetActiveManager on the page, but for some reason the call to nsEventStateManager::ClearGlobalActiveContent isn't happening, so the PresShell believes the page is still active even though the alert is displayed.

Comment 16

[Neil Deakin]

Attachment: testcase

Actually, just showing an alert during a mouseup event causes this bug.

Perhaps ClearGlobalActiveContent just needs to be called from nsGlobalWindow::EnterModalState?

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

Yeah, something like that could work.
ClearGlobalActiveContent should be probably called only if
globalwindow or any of its ancestor or descendant window is focused.

Comment 18

[Neil Deakin]

(In reply to comment #17)
> Yeah, something like that could work.
> ClearGlobalActiveContent should be probably called only if
> globalwindow or any of its ancestor or descendant window is focused.

Why is the focus relevant?

Comment 19

[Olli Pettay [:smaug][[e-mail]]]

oops, not focus, but currently active ESM

Comment 20

[Neil Deakin]

Attachment: patch

Comment 21

[Raul Nicolae Malea]

Reconfirm this bug for: Mozilla/5.0 (Windows NT 6.1; rv:2.0b9pre) Gecko/20110106 Firefox/4.0b9pre ID:20110106030349

Comment 22

[Olli Pettay [:smaug][[e-mail]]]

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


>+  // If there is an active ESM in this window, clear it. Otherwise, this can
>+  // cause a problem if a modal state is entered during a mouseup event.
>+  nsEventStateManager* activeESM =
>+    static_cast<nsEventStateManager*>(nsEventStateManager::GetActiveEventStateManager());
>+  if (activeESM && activeESM->GetPresContext()) {
>+    nsIPresShell* activeShell = activeESM->GetPresContext()->GetPresShell()
>+    nsCOMPtr<nsIDocument> doc = do_QueryInterface(mDocument);
You could just use mDoc.


>+    if (activeShell && (
>+        nsContentUtils::ContentIsCrossDocDescendantOf(activeShell->GetDocument(), doc) ||
>+        nsContentUtils::ContentIsCrossDocDescendantOf(doc, activeShell->GetDocument()))) {
>+      nsEventStateManager::ClearGlobalActiveContent(nsnull);
I think you should pass activeESM as parameter, so that whatever is ACTIVE in that document,
gets cleared.


Could you still try what happens if web page calls window.print(). 
Things should be just ok in that case, but better to test.

Comment 23

[Neil Deakin]

Attachment: patch, address comments, for checkin

Comment 24

[Justin Dolske [:Dolske]]

Pushed http://hg.mozilla.org/mozilla-central/rev/5e8b96f85355

Thanks for grabbing this and fixing so fast!

Comment 25

[Justin Dolske [:Dolske]]

Oops, test was broken.

Disabled in http://hg.mozilla.org/mozilla-central/rev/534adf40eb8b
Fixed and reenabled in http://hg.mozilla.org/mozilla-central/rev/7b08fef26335

Just needed to include EventUtils.js, but I left some of the extra debugging in lest the test ever cause problems again.