263545 - After a javascript click, the spacebar no longer pages down.

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Joe Smith]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20040913
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20040913

It looks to be an interaction with Javascript that causes the spacebar to no
longer scroll down the page after clicking on an image.

Reproducible: Always
Steps to Reproduce:
1. Go to a CNN story that has an embedded image gallery (see URL).
2. Click on the yellow Next button below the embedded picture.
3. After the new image loads, try using spacebar to scroll down.

Actual Results:  
Even if you force a focus change by click-and-drag on some text, spacebar gets
ignored.  It remains disabled until the mouse is clicked in a text box (such as
"Search CNN.com") and then clicked back on the main page.

Expected Results:  
Continue to operate the way it had before the image was clicked.

Comment 1

[Boris Zbarsky [:bzbarsky]]

I see the bug with a current build too...  Would be nice to have a minimal testcase.

Comment 2

[Mats Palmgren (inactive)]

Attachment: Testcase

Comment 3

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

Comment 4

[Boris Zbarsky [:bzbarsky]]

Hmm....  Handling user input could include a mousemove event, no?  As in, if
something is focused and we move the mouse and the mousemove removes the focused
node from the document.... Should we fire blur then?  If so, why?

I think there was a recent post about this issue to n.p.m.dom.  Perhaps removing
nodes from the document should just generally fire blur event handlers?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 162263 [details] [diff] [review]
Patch rev. 1

Marking r- per comments.

Comment 6

[Mats Palmgren (inactive)]

(In reply to comment #4)
> Hmm....  Handling user input could include a mousemove event, no?

No, IsHandlingUserInput() returns false for mouse move events.
The "usr input" state is handled by nsAutoHandlingUserInputStatePusher
which is used from
content/html/content/src/nsHTMLFormElement.cpp
  - for submit events
layout/base/nsPresShell.cpp
  - button/key events + focus/activate if origin is chrome

> Perhaps removing nodes from the document should just generally
> fire blur event handlers?

I don't think so.
http://www.w3.org/TR/DOM-Level-2-Events/events.html
"The blur event occurs when an element loses focus either via
the pointing device or by tabbing navigation."

Comment 7

[Boris Zbarsky [:bzbarsky]]

This doesn't handle the case when the removed node is an ancestor of the 
mCurrentFocus, does it?

Comment 8

[Mats Palmgren (inactive)]

Attachment: Patch rev. 2

It was impractical to keep this separate from bug 275621 so I combined them.
I think we have the same problem with the other content pointers, so I fixed
them too.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Looks reasonable, but I think use of nsContentUtils::ContentIsDescendantOf would make 
it more readable... (note the comments about passing null to that method, though!)

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 169339 [details] [diff] [review]
Patch rev. 2

r- based on comment 9.

Comment 11

[Wayne Mery (:wsmwk)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050511
Firefox/1.0.4

comment 0 affects me as described, HOWEVER, testcase 2004-10-15 doesn't entire,
at least as the testcase text describes - spacebar doesn't work at all from the
moment I click the testcase link.

Does this bug also fix my issue (which is how I arrived here)?  Which is ... at
bottom of
http://www.cnn.com/2004/TECH/space/10/04/spaceshipone.attempt.cnn/index.html
middle click to open in a new tab "Click Here to try 4 Free Trial Issues of
Time!" - spacebar is not functional.

Comment 12

[Mats Palmgren (inactive)]

This was fixed in the interval 20090609 -- 20090611, the new focus manager
(bug 178324) landed in that interval.  Note that spacebar scrolls the page
in *both* cases in Testcase.  I believe the new behavior is correct.

-> FIXED (by bug 178324)

Comment 13

[Boris Zbarsky [:bzbarsky]]

Need a regression test.