307172 - Focus never returns when a link having focus is hidden

Status: RESOLVED FIXED

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

Description

[malika.jaiswal]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.1) Gecko/20040707
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.1) Gecko/20040707

When a Link having focus is hidden the focus never comes back and the keyboard
keys don't work.

Reproducible: Always

Steps to Reproduce:
A Testcase is provide for this .
1. Open the file Testcase.htm
2. Press "A" to get the hidden link.
3. After this press "Esc"
4. Try to use keyboard keys e.g Press "ctrl+O"

Actual Results:  
1. On Pressing "A" a hidden link is seen with focus on it.
2. On Pressing "Esc" the link is hidden.
3. On Pressing "Ctrl+O" nothing happens i.e, the Open File window does not open.

None of the keyboard shortcuts work

Expected Results:  
On Pressing "Ctrl+O" the Open File window should open. 
All the keyborad shortcuts should work.

Comment 1

[malika.jaiswal]

Attachment: Testcase

Comment 2

[KIRAN SHIVARAMA SUNDAR]

Confirming the bug on FireFox 1.06. The focus seems to be lost once the link is
hidden after pressing escape

Comment 3

[zug_treno]

Reproducible with Mozilla 1.8b1, Deer Park a2 and SeaMonkey/20050904, related
to/duplicate of Core bug 255317?

Comment 4

[malika.jaiswal]

This problem seems to be happening from Mozilla 1.7RC1(Released April 21, 2004)
and its working fine till Mozilla 1.7 Beta (Released March 18, 2004).

Comment 5

[Aaron Leventhal]

> its working fine till Mozilla 1.7 Beta (Released March 18, 2004).
Can someone confirm that? I don't think this ever worked right. I believe this
has been a known issue for some time -- the focus code just never dealt with it
properly.

See bug 255187 -- this is probably a dupe of that.

I suggest we offer the workaround of tracking the focus and storing it in a
variable. When hiding something with focus, using gLastFocus.blur() before
actually hiding it. It could be that using timeouts is necessary either before
the blur, or before hiding the element, or both.

Comment 6

[malika.jaiswal]

> Can someone confirm that? I don't think this ever worked right. 

With the given Testcase I am able to recreate the problem in Mozilla 1.7 Beta
but it fails in Mozilla 1.7 RC1.

Comment 7

[zug_treno]

WFM with Mozilla 1.7b (and Mozilla 1.6) but reproducible with Mozilla
1.7b/20040409 (and Mozilla 1.7RC1).

Comment 8

[ABHIJEET BHATTACHARYA]

Fix given for bugid #214373 is breaking. I think we should fix this bug original
bug.

Comment 9

[Aaron Leventhal]

Bryner, I can confirm that bug 214373 caused this. This is a problem in branch,
trunk and 1.7.x

Removing this line from PresShell::HandleEvent() fixes things
-      esm->GetFocusedContent(getter_AddRefs(mCurrentEventContent));

Comment 10

[Aaron Leventhal]

Attachment: Don't get current event content if current event frame is null. Patch for trunk and MOZILLA_1_8_BRANCH .... patch for 1.7 would be similar

This fixes the regression without breaking the fix for bug 214373. Brian,
please check it carefully since it'd be good to get in on the branches.

Comment 11

[ABHIJEET BHATTACHARYA]

I have tested the fix provided by Aaron. Its working fine on OS/2. confirming
that its not breaking fix for 214373.

Comment 12

[Brian Ryner (not reading)]

Comment on attachment 197141 [details] [diff] [review]
Don't get current event content if current event frame is null. Patch for trunk and MOZILLA_1_8_BRANCH  .... patch for 1.7 would be similar

This should be ok, and I think it matches the original intent of the code.

Comment 13

[Aaron Leventhal]

Fixed on trunk. Not fixed on either the 1.7 or 1.8 branch yet.

Should I mark it fixed and then we add fixed1.7 and fixed1.8 keywords later?

Comment 14

[Mike Kaply [:mkaply]]

Comment on attachment 197141 [details] [diff] [review]
Don't get current event content if current event frame is null. Patch for trunk and MOZILLA_1_8_BRANCH  .... patch for 1.7 would be similar

a=mkaply since this is a regression

Comment 15

[[e-mail]]

Does this mean that this change can be backed out?

https://bugzilla.mozilla.org/attachment.cgi?oldid=62198&action=interdiff&newid=62508&headers=1

Comment 16

[Aaron Leventhal]

This is fixed now on trunk and 1.8 branch.

Mike said he would fix this on the 1.7 branch. Mike will you close this when
you're done with that?

Comment 17

[Aaron Leventhal]

This caused bug 310350. Recommend backing out until we can find a safe fix.

Comment 18

[Aaron Leventhal]

(In reply to comment #17)
> This caused bug 310350. Recommend backing out until we can find a safe fix.

Apologies, it looks like this was not the cause of bug 310350.

Comment 19

[Robi Brunner]

Testcase fails to work if typeaheadfind is turned on.  Shouldn't in page key controls have priority over typeahead search?

Comment 20

[Aaron Leventhal]

This is fixed in the trunk and on the 1.8 branch. We don't need to take the fix on any other branch.

Comment 21

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1527381