739874 - Input type "password" doesn't receive Focus event with a mouse click

Status: NEW

(Core :: DOM: Events, defect)

Description

[Simon de Almeida]

Attachment: input_bug.html

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/535.11 (KHTML, like Gecko) Chrome/17.0.963.83 Safari/535.11

Steps to reproduce:

1. Add a Focus and a Click Listener on an input element with a type="password" attribute.
2. Set its default value to anything ( Must be at least 1 Char long ).
3. Click on the displayed bullets ( Hidden Text ).


Actual results:

Only the Click event was fired, but not the Focus event. If clicked again, then both Click and Focus event were fired as expected.


Expected results:

On the first time the input received a Click event, the Focus event should have been dispatched.

*If clicked on the white space on the input element by the first time, the click and focus event were dispatched as expected.

It seems that the problem only occurs when clicking on the password bullets.

Comment 1

[Simon de Almeida]

If possible, I would like to take this as my first bug. I've been wanting to contribute to Firefox and I think this would be a great starting bug.

Comment 2

[Tim (fmdeveloper)]

Confirmed on Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20120328 Firefox/14.0a1 ID:20120328031211

Moving to CORE:DOM:Events for insight

Comment 3

[Boris Zbarsky [:bzbarsky]]

Could this be lazy editor init or something?

Comment 4

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

yeah. Are we generating the contents of input element when it is clicked.
That could affect to the target of the event. I mean, mousedown goes to some node, but mouseup to
some other node. though, I thought that case has been fixed already few times.
Masayuki might remember

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, but according to comment 0, click event is fired correctly, but focus isn't moved...

Comment 6

[(no longer active)]

So here's what's happening today.  When you click on the textframe inside the password field, nsEventStateManager::PreHandleEvent is called with aFrame set to the textframe, which is assigned to mCurrentTarget.  Then, when we initialize the editor, we end up destroying the text frame which clears mCurrentTarget like this:

#0  nsWeakFrame::Clear (this=0x123802d30, aShell=0x11ae40c00) at nsIFrame.h:3066
#1  0x00000001016f3b17 in PresShell::ClearFrameRefs (this=0x11ae40c00, aFrame=0x11ad601f8) at /Users/ehsanakhgari/moz/mozilla-central/layout/base/nsPresShell.cpp:2920
#2  0x000000010178c11e in nsFrame::DestroyFrom (this=0x11ad601f8, aDestructRoot=0x11ad601f8) at /Users/ehsanakhgari/moz/mozilla-central/layout/generic/nsFrame.cpp:650
#3  0x000000010182a40c in nsTextFrame::DestroyFrom (this=0x11ad601f8, aDestructRoot=0x11ad601f8) at /Users/ehsanakhgari/moz/mozilla-central/layout/generic/nsTextFrameThebes.cpp:3844
#4  0x000000010165834e in nsIFrame::Destroy (this=0x11ad601f8) at nsIFrame.h:585
#5  0x000000010176c9c5 in nsBlockFrame::DoRemoveFrame (this=0x11ad60020, aDeletedFrame=0x11ad601f8, aFlags=2) at /Users/ehsanakhgari/moz/mozilla-central/layout/generic/nsBlockFrame.cpp:5525
#6  0x000000010176be7f in nsBlockFrame::RemoveFrame (this=0x11ad60020, aListID=mozilla::layout::kPrincipalList, aOldFrame=0x11ad601f8) at /Users/ehsanakhgari/moz/mozilla-central/layout/generic/nsBlockFrame.cpp:5079
#7  0x00000001016ba146 in nsFrameManager::RemoveFrame (this=0x11ae42400, aListID=mozilla::layout::kPrincipalList, aOldFrame=0x11ad601f8, aInvalidate=true) at /Users/ehsanakhgari/moz/mozilla-central/layout/base/nsFrameManager.cpp:534
#8  0x000000010164f732 in nsCSSFrameConstructor::ContentRemoved (this=0x11ae42400, aContainer=0x1238b8e80, aChild=0x1238b8f00, aOldNextSibling=0x0, aFlags=nsCSSFrameConstructor::REMOVE_CONTENT, aDidReconstruct=0x7fff5fbfa39f) at /Users/ehsanakhgari/moz/mozilla-central/layout/base/nsCSSFrameConstructor.cpp:7477
#9  0x00000001016f847c in PresShell::ContentRemoved (this=0x11ae40c00, aDocument=0x123a60000, aContainer=0x1238b8e80, aChild=0x1238b8f00, aIndexInContainer=0, aPreviousSibling=0x0) at /Users/ehsanakhgari/moz/mozilla-central/layout/base/nsPresShell.cpp:4243
#10 0x00000001016f84ff in non-virtual thunk to PresShell::ContentRemoved(nsIDocument*, nsIContent*, nsIContent*, int, nsIContent*) () at /Users/ehsanakhgari/moz/mozilla-central/layout/base/nsPresShell.cpp:4248
#11 0x0000000101b712a9 in nsNodeUtils::ContentRemoved (aContainer=0x1238b8e80, aChild=0x1238b8f00, aIndexInContainer=0, aPreviousSibling=0x0) at /Users/ehsanakhgari/moz/mozilla-central/content/base/src/nsNodeUtils.cpp:196
#12 0x0000000101b44d1e in nsINode::doRemoveChildAt (this=0x1238b8e80, aIndex=0, aNotify=true, aKid=0x1238b8f00, aChildArray=@0x1238b8ef0) at /Users/ehsanakhgari/moz/mozilla-central/content/base/src/nsGenericElement.cpp:3887
#13 0x0000000101b44b96 in nsGenericElement::RemoveChildAt (this=0x1238b8e80, aIndex=0, aNotify=true) at /Users/ehsanakhgari/moz/mozilla-central/content/base/src/nsGenericElement.cpp:3858
#14 0x0000000101b35b76 in nsINode::RemoveChild (this=0x1238b8e80, aOldChild=0x1238b8f00) at /Users/ehsanakhgari/moz/mozilla-central/content/base/src/nsGenericElement.cpp:538
#15 0x0000000101b35dfe in nsINode::RemoveChild (this=0x1238b8e80, aOldChild=0x1238b8f78, aReturn=0x7fff5fbfa780) at /Users/ehsanakhgari/moz/mozilla-central/content/base/src/nsGenericElement.cpp:566
#16 0x0000000101b1be90 in nsGenericElement::RemoveChild (this=0x1238b8e80, aOldChild=0x1238b8f78, aReturn=0x7fff5fbfa780) at nsGenericElement.h:432
#17 0x0000000101d73260 in nsHTMLDivElement::RemoveChild (this=0x1238b8e80, oldChild=0x1238b8f78, _retval=0x7fff5fbfa780) at /Users/ehsanakhgari/moz/mozilla-central/content/html/content/src/nsHTMLDivElement.cpp:60
#18 0x0000000101d73d17 in non-virtual thunk to nsHTMLDivElement::RemoveChild(nsIDOMNode*, nsIDOMNode**) () at /Users/ehsanakhgari/moz/mozilla-central/content/html/content/src/nsHTMLDivElement.cpp:66
#19 0x000000010228b3c5 in DeleteElementTxn::DoTransaction (this=0x11ad116a0) at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/base/DeleteElementTxn.cpp:148
#20 0x000000010228f6fa in EditAggregateTxn::DoTransaction (this=0x11ad5d3c0) at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/base/EditAggregateTxn.cpp:70
#21 0x000000010228cd8e in DeleteRangeTxn::DoTransaction (this=0x11ad5d3c0) at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/base/DeleteRangeTxn.cpp:195
#22 0x000000010228f6fa in EditAggregateTxn::DoTransaction (this=0x11ae2f430) at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/base/EditAggregateTxn.cpp:70
#23 0x00000001029f7904 in nsTransactionItem::DoTransaction (this=0x11ad54f70) at /Users/ehsanakhgari/moz/mozilla-central/editor/txmgr/src/nsTransactionItem.cpp:213
#24 0x00000001029fd3ba in nsTransactionManager::BeginTransaction (this=0x1253ba620, aTransaction=0x11ae2f430) at /Users/ehsanakhgari/moz/mozilla-central/editor/txmgr/src/nsTransactionManager.cpp:771
#25 0x00000001029faf97 in nsTransactionManager::DoTransaction (this=0x1253ba620, aTransaction=0x11ae2f430) at /Users/ehsanakhgari/moz/mozilla-central/editor/txmgr/src/nsTransactionManager.cpp:105
#26 0x0000000102255e49 in nsEditor::DoTransaction (this=0x11d12f0c0, aTxn=0x11ae2f430) at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/base/nsEditor.cpp:727
#27 0x0000000102267491 in nsEditor::DeleteSelectionImpl (this=0x11d12f0c0, aAction=0) at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/base/nsEditor.cpp:4327
#28 0x000000010224d67f in nsTextEditRules::WillDeleteSelection (this=0x11ad5f180, aSelection=0x124d73c80, aCollapsedAction=0, aCancel=0x7fff5fbfb0b7, aHandled=0x7fff5fbfb0b6) at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/text/nsTextEditRules.cpp:877
#29 0x000000010224b86e in nsTextEditRules::WillDoAction (this=0x11ad5f180, aSelection=0x124d73c80, aInfo=0x7fff5fbfb0b8, aCancel=0x7fff5fbfb0b7, aHandled=0x7fff5fbfb0b6) at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/text/nsTextEditRules.cpp:273
#30 0x0000000102241db0 in nsPlaintextEditor::DeleteSelection (this=0x11d12f0c0, aAction=0) at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/text/nsPlaintextEditor.cpp:789
#31 0x000000010224c1e8 in nsTextEditRules::WillInsertText (this=0x11ad5f180, aAction=2000, aSelection=0x124d73c80, aCancel=0x7fff5fbfb5cf, aHandled=0x7fff5fbfb5ce, inString=0x7fff5fbfb940, outString=0x7fff5fbfb620, aMaxLength=-1) at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/text/nsTextEditRules.cpp:636
#32 0x000000010224b843 in nsTextEditRules::WillDoAction (this=0x11ad5f180, aSelection=0x124d73c80, aInfo=0x7fff5fbfb5d0, aCancel=0x7fff5fbfb5cf, aHandled=0x7fff5fbfb5ce) at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/text/nsTextEditRules.cpp:265
#33 0x000000010224224b in nsPlaintextEditor::InsertText (this=0x11d12f0c0, aStringToInsert=@0x7fff5fbfb940) at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/text/nsPlaintextEditor.cpp:836
#34 0x000000010224240f in non-virtual thunk to nsPlaintextEditor::InsertText(nsAString_internal const&) () at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/text/nsPlaintextEditor.cpp:848
#35 0x0000000101d3eb77 in nsTextEditorState::SetValue (this=0x11d9da230, aValue=@0x7fff5fbfbd08, aUserInput=false) at /Users/ehsanakhgari/moz/mozilla-central/content/html/content/src/nsTextEditorState.cpp:1849
#36 0x0000000101d3caed in nsTextEditorState::PrepareEditor (this=0x11d9da230, aValue=0x0) at /Users/ehsanakhgari/moz/mozilla-central/content/html/content/src/nsTextEditorState.cpp:1329
#37 0x0000000101dba23a in nsHTMLInputElement::CreateEditor (this=0x12559f720) at /Users/ehsanakhgari/moz/mozilla-central/content/html/content/src/nsHTMLInputElement.cpp:1201
#38 0x0000000101dba27c in non-virtual thunk to nsHTMLInputElement::CreateEditor() () at /Users/ehsanakhgari/moz/mozilla-central/content/html/content/src/nsHTMLInputElement.cpp:1204
#39 0x000000010173e9aa in nsTextControlFrame::EnsureEditorInitialized (this=0x11d52e3e0) at /Users/ehsanakhgari/moz/mozilla-central/layout/forms/nsTextControlFrame.cpp:376
#40 0x000000010173ee1c in non-virtual thunk to nsTextControlFrame::EnsureEditorInitialized() () at /Users/ehsanakhgari/moz/mozilla-central/layout/forms/nsTextControlFrame.cpp:359
#41 0x0000000101dbbe64 in nsHTMLInputElement::PreHandleEvent (this=0x12559f720, aVisitor=@0x7fff5fbfc300) at /Users/ehsanakhgari/moz/mozilla-central/content/html/content/src/nsHTMLInputElement.cpp:1799
#42 0x0000000101ce438d in nsEventTargetChainItem::PreHandleEvent (this=0x11c71b428, aVisitor=@0x7fff5fbfc300) at /Users/ehsanakhgari/moz/mozilla-central/content/events/src/nsEventDispatcher.cpp:277
#43 0x0000000101ce5918 in nsEventDispatcher::Dispatch (aTarget=0x1238b8e80, aPresContext=0x123f63000, aEvent=0x7fff5fbfd1b8, aDOMEvent=0x0, aEventStatus=0x7fff5fbfcf24, aCallback=0x7fff5fbfc620, aTargets=0x0) at /Users/ehsanakhgari/moz/mozilla-central/content/events/src/nsEventDispatcher.cpp:656
#44 0x0000000101700498 in PresShell::HandleEventInternal (this=0x11ae40c00, aEvent=0x7fff5fbfd1b8, aStatus=0x7fff5fbfcf24) at /Users/ehsanakhgari/moz/mozilla-central/layout/base/nsPresShell.cpp:6564
#45 0x00000001016ff721 in PresShell::HandlePositionedEvent (this=0x11ae40c00, aTargetFrame=0x11ad601f8, aEvent=0x7fff5fbfd1b8, aEventStatus=0x7fff5fbfcf24) at /Users/ehsanakhgari/moz/mozilla-central/layout/base/nsPresShell.cpp:6237
#46 0x00000001016fea50 in PresShell::HandleEvent (this=0x11c767000, aFrame=0x11c79dc38, aEvent=0x7fff5fbfd1b8, aDontRetargetEvents=false, aEventStatus=0x7fff5fbfcf24) at /Users/ehsanakhgari/moz/mozilla-central/layout/base/nsPresShell.cpp:6064
#47 0x0000000102009fc4 in nsViewManager::DispatchEvent (this=0x11aee9f60, aEvent=0x7fff5fbfd1b8, aView=0x11c4d1600, aStatus=0x7fff5fbfcf24) at /Users/ehsanakhgari/moz/mozilla-central/view/src/nsViewManager.cpp:908
#48 0x0000000102005669 in HandleEvent (aEvent=0x7fff5fbfd1b8) at /Users/ehsanakhgari/moz/mozilla-central/view/src/nsView.cpp:158
#49 0x0000000102d864e2 in nsChildView::DispatchEvent (this=0x100395430, event=0x7fff5fbfd1b8, aStatus=@0x7fff5fbfd08c) at /Users/ehsanakhgari/moz/mozilla-central/widget/cocoa/nsChildView.mm:1512
#50 0x0000000102d86576 in nsChildView::DispatchWindowEvent (this=0x100395430, event=@0x7fff5fbfd1b8) at /Users/ehsanakhgari/moz/mozilla-central/widget/cocoa/nsChildView.mm:1522
#51 0x0000000102d8d9e0 in -[ChildView mouseDown:] (self=0x11c408680, _cmd=0x7fff886b9568, theEvent=0x11d168e80) at /Users/ehsanakhgari/moz/mozilla-central/widget/cocoa/nsChildView.mm:3290
#52 0x00007fff880b13a7 in -[NSWindow sendEvent:] ()
#53 0x0000000102d7ee95 in -[ToolbarWindow sendEvent:] (self=0x100394080, _cmd=0x7fff886af2c0, anEvent=0x11d168e80) at /Users/ehsanakhgari/moz/mozilla-central/widget/cocoa/nsCocoaWindow.mm:2634
#54 0x00007fff87fe6afa in -[NSApplication sendEvent:] ()
#55 0x0000000102d6db9b in -[GeckoNSApplication sendEvent:] (self=0x118fe20d0, _cmd=0x7fff886af2c0, anEvent=0x11d168e80) at /Users/ehsanakhgari/moz/mozilla-central/widget/cocoa/nsAppShell.mm:196
#56 0x00007fff87f7d6de in -[NSApplication run] ()
#57 0x0000000102d6fedc in nsAppShell::Run (this=0x100328fc0) at /Users/ehsanakhgari/moz/mozilla-central/widget/cocoa/nsAppShell.mm:795
#58 0x0000000102a0c2c6 in nsAppStartup::Run (this=0x118fdd650) at /Users/ehsanakhgari/moz/mozilla-central/toolkit/components/startup/nsAppStartup.cpp:295
#59 0x0000000101254fde in XRE_main (argc=5, argv=0x7fff5fbff098, aAppData=0x1000081e0) at /Users/ehsanakhgari/moz/mozilla-central/toolkit/xre/nsAppRunner.cpp:3706
#60 0x0000000100001d14 in do_main (argc=5, argv=0x7fff5fbff098) at /Users/ehsanakhgari/moz/mozilla-central/browser/app/nsBrowserApp.cpp:190
#61 0x00000001000015dc in main (argc=5, argv=0x7fff5fbff098) at /Users/ehsanakhgari/moz/mozilla-central/browser/app/nsBrowserApp.cpp:277

Then when nsEventStateManager::PostHandleEvent is called, mCurrentTarget is null, so the focus event will not get dispatched.

I'm not sure what the best way to fix this would look like.  One possible fix would be to make nsHTMLInputElement::NeedToInitializeEditorForEvent ignore mouse down events, but I really don't like that solution.  Boris, smaug, is there a better way to fix this?

Comment 7

[(no longer active)]

But I'm not quite sure why this only happens for password inputs yet...

Comment 8

[Boris Zbarsky [:bzbarsky]]

I thought we initialized on mousedown so that the click would position the caret correctly...

Figuring out why this only happens for password inputs is interesting.  Do we reuse the same textframe (and presumably textnode) for other inputs?  Can we do it for password inputs?

Comment 9

[(no longer active)]

I may have an idea why the behavior is different for password inputs.  This code <http://dxr.lanedo.com/mozilla-central/content/html/content/src/nsTextEditorState.cpp.html?string=nsTextEditorState#l1751> retrieves the current value of the text node, which we then compare to the value passed to SetValue to determine if we need to update the textnode value.  For a password input, the current value is just a bunch of asterisks, which causes us to set the value using the editor, which results in the text node to be deleted.

Still this doesn't bring us closer to figuring out how to fix the bug...

Comment 10

[Boris Zbarsky [:bzbarsky]]

> which we then compare to the value passed to SetValue to determine if we need to update
> the textnode value.

Should we be doing the transformation passwords do _before_ doing that compare, perhaps?

Comment 11

[(no longer active)]

(In reply to Boris Zbarsky (:bz) from comment #10)
> > which we then compare to the value passed to SetValue to determine if we need to update
> > the textnode value.
> 
> Should we be doing the transformation passwords do _before_ doing that
> compare, perhaps?

I don't follow this comment...

Comment 12

[Boris Zbarsky [:bzbarsky]]

The issue we're running into is that SetValue is passed some string while the current value is asterisks.  So we decide they're not equal, and do something in the editor that converts the value passed into asterisks and then sets it.

Can we do the conversion of the passed-in value to asterisks before doing the compare to the current value?

Comment 13

[(no longer active)]

Oh, right.  OK, I'll try that.

Comment 14

[(no longer active)]

Attachment: Patch (v1)

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 613347 [details] [diff] [review]
Patch (v1)

r=me if the actual value submitted for the password is unaffected.

Comment 16

[(no longer active)]

Comment on attachment 613347 [details] [diff] [review]
Patch (v1)

Ah, this won't work, since later on we try to grab the value from the editor again...

Comment 17

[(no longer active)]

In fact, I don't think we can fix the problem this way, since the fact of the matter is that the editor needs to see the new value at some point, and it will go on and rewrite the textnode when it sees it.  :(