Closed Bug 1305458 Opened 8 years ago Closed 8 years ago

Changing -moz-appearence on hover breaks change event

Categories

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

Product:
Core
Component:
DOM: UI Events & Focus Handling
Version:
42 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox53 --- fixed

People

(Reporter: dspeyer, Assigned: stone)

Details

Attachments

(15 files, 35 obsolete files)

1305458.html
473 bytes, text/html
Details
Part1: Rename nsIDOMEventTarget::PreHandleEvent to nsIDOMEventTarget::GetEventTargetParent. r=smaug
78.89 KB, patch
stone
: review+
Details | Diff | Splinter Review
Part2: Add nsIDOMEventTarget::PreHandleEvent. r=smaug
7.70 KB, patch
stone
: review+
Details | Diff | Splinter Review
Part3: Add EventTargetChainItem::GetFirstEventTarget. r=smaug
4.96 KB, patch
stone
: review+
Details | Diff | Splinter Review
Part4: Call EventTargetChainItem::PreHandleEvent even it sets mCanHandle=false. r=smaug
10.25 KB, patch
stone
: review+
Details | Diff | Splinter Review
Part5: Move form control frame focus/blur from nsGenericHTMLFormElement::GetEventTargetParent to PreHandleEvent. r=smaug
3.24 KB, patch
stone
: review+
Details | Diff | Splinter Review
Part6: Move fire change event from HTMLTextAreaElement::GetEventTargetParent to PreHandleEvent. r=smaug
2.75 KB, patch
stone
: review+
Details | Diff | Splinter Review
Part7: Refine nsXULElement::GetEventTargetParent. r=smaug
7.41 KB, patch
stone
: review+
Details | Diff | Splinter Review
Part8: Move dispatch XUL command from nsXULElement::GetEventTargetParent to PreHandleEvent. r=smaug
4.98 KB, patch
stone
: review+
Details | Diff | Splinter Review
Part9: Move fire events and set value from HTMLInputElement::GetEventTargetParent to PreHandleEvent. r=smaug
7.87 KB, patch
stone
: review+
Details | Diff | Splinter Review
Bug 1305458 Part10: Add test case. r=smaug
2.49 KB, patch
stone
: review+
Details | Diff | Splinter Review
Bug 1305458 Part11: Let HTMLInputElement delegate event handling to it's parent class. r=smaug
2.12 KB, patch
stone
: review+
Details | Diff | Splinter Review
Bug 1305458 Part12: Refine EventTargetChain flags to reduce overheads. r=smaug
5.98 KB, patch
stone
: review+
Details | Diff | Splinter Review
Bug 1305458 Part13: Refine event target chain creation. r=smaug
5.70 KB, patch
stone
: review+
Details | Diff | Splinter Review
Bug 1305458 Part14: Refine assertion in EventTargetChainItem::Create r=smaug
1.89 KB, patch
stone
: review+
Details | Diff | Splinter Review
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:42.0) Gecko/20100101 Firefox/42.0 Build ID: 20151029151421 Steps to reproduce: Open the following webpage: <html> <head> <script src="https://ajax.googleapis.com/ajax/libs/jquery/2.2.0/jquery.min.js"></script> <style> input[type=number] { -moz-appearance: textfield; } input[type=number]:focus, input[type=number]:hover { -moz-appearance: number-input; } </style> </head> <body> <input id="foo" type="number"> <script> $('#foo').on('change', function(){ $('body').append('X'); }); </script> </body> </html> Click on the input, leave the mouse there, enter a number, hit tab. Click on the input, take the mouse out, enter a number, hit tab. Actual results: An "X" appeared on the page only the first time Expected results: "X"s should have appeared on the page both times.
Attached file 1305458.html —
Component: Untriaged → Event Handling
Product: Firefox → Core
Hi Stone, are you interested in taking a look?
Flags: needinfo?(sshih)
Priority: -- → P3
Assignee: nobody → sshih
Flags: needinfo?(sshih)
Hover state changes will induce style changes. When anonymous input (text) element pre-handles blur event (user hit tap), it [1] delegates event to nsGenericHTMLFormElement::PreHandleEvent which gets its from control frame [2] and then induce flushing pending restyles [3]. After flush pending notifications, the nsAutoScriptBlocker in [4] is destructed, the blocked runnables are executed. The old anonymous elements are unbind from tree. When the call stack pop back to EventDispatcher::Dispatch, the event is stopped propagating to the non-anonymous input (number) element. That's why the listener is not triggered. [1] http://searchfox.org/mozilla-central/source/dom/html/HTMLInputElement.cpp#3880 [2] http://searchfox.org/mozilla-central/source/dom/html/nsGenericHTMLElement.cpp#2204 [3] http://searchfox.org/mozilla-central/source/dom/html/nsGenericHTMLElement.cpp#1014 [4] http://searchfox.org/mozilla-central/source/layout/base/nsPresShell.cpp#4115
When user hits tab with mouse doesn't hover on input Anonymous input element PreHandleEvent function handles blur event and fire change event on itself Anonymous input element PreHandleEvent function trigger flushing pending restyles Anonymous elements unbind from tree Call stack pop back to EventDispatcher::Dispatch (dispatch blur event) and continue ask the parent of anonymous input element to handle the blur event. At this moment, the anonymous input element is unbind from tree so the non-anonymous input element won't handle the blur event and have no chance to fire change event.
The pending restyles are caused in [1]. Trying to flush pending styles earlier [2] and it will induce the NAC unbind from tree earlier while the target of blur event is already set to the anonymous input element. Not work for this bug. I'm wondering what is the style applied to frames should be when content handling focus / blur events? When element is blurred, if we update its style before dispatching event, the event target may be unbind from tree. Or we have to recalculate the event target. If we delay flushing pending styles after dispatching blur event. Get styles in the listeners of content will return styles before blur happened. [1] https://dxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp#1663 [2] https://dxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp#1711
Whenever JS accesses some styling information, layout is flushed. For flushing earlier I was thinking something like keep a strong reference to the first non-NAC element, and if flushing removes NAC, target focus changes to the non-NAC element.
In the case of blur, nsFocusManager::Blur triggers NotifyFocusStateChange (triggers pending restyles) and SendFocusOrBlurEvent. That means we have to flush pending restyles after NotifyFocusStateChange and before SendFocusOrBlurEvent. We may NotifyFocusStateChange on NAC element and fire blur event to non-NAC element when the NAC element is unbind from tree. In the case of focus, it's similar to blur but additionally change focus to the non-NAC element. Is my understanding correct?
yeah. That is something I'd try, unless there are other better options. One other possibility is to add a new callback param to EventDispatcher::Dispatch which would be called right after creating event target chain but before handling the event, and flush layout using that callback.
Collected ::PreHandleEvent which may fire events or flush pending restyles HTMLInputElement::PreHandleEvent May fire change event in [1] HTMLButtonElement::PreHandleEvent May notify form observers in [2] and trigger js code [3] HTMLTextAreaElement::PreHandleEvent May fire change event when handling blur event [4] nsGenericHTMLFormElement::PreHandleEvent May flush pending restyles in [5] [1] http://searchfox.org/mozilla-central/source/dom/html/HTMLInputElement.cpp#3933 [2] http://searchfox.org/mozilla-central/source/dom/html/HTMLFormElement.cpp#1588 [3] http://searchfox.org/mozilla-central/source/toolkit/components/satchel/formSubmitListener.js#119 [4] http://searchfox.org/mozilla-central/source/dom/html/HTMLTextAreaElement.cpp#539 [5] http://searchfox.org/mozilla-central/source/dom/html/nsGenericHTMLElement.cpp#2204
Considering the case that an element wants PreHandleEvent but set mCanHandle=false, we can't destroy the EventTargetChainItem immediately. So I keep the EventTargetChainItem in the event target chain (if mCanHandle=true) and call its PreHandleEvent after the event target chain is created. And also ignore dispatching event to its listener.
Attachment #8805408 - Attachment is obsolete: true
Tested performance overheads with [1] and the results are before: avg of 30 results (total tests 50) : 102.96666666666667ms after: avg of 30 results (total tests 50) : 113.16666666666667ms [1] http://mozilla.pettay.fi/moztests/events/event_speed_2.1_nolistener.html
Attachment #8805405 - Flags: review?(bugs)
Attachment #8805406 - Flags: review?(bugs)
Attachment #8805407 - Flags: review?(bugs)
Attachment #8805486 - Flags: review?(bugs)
Attachment #8805409 - Flags: review?(bugs)
Attachment #8805410 - Flags: review?(bugs)
Attachment #8805411 - Flags: review?(bugs)
Attachment #8805412 - Flags: review?(bugs)
Attachment #8805413 - Flags: review?(bugs)
Attachment #8805414 - Flags: review?(bugs)
(In reply to Ming-Chou Shih [:stone] from comment #22) > Tested performance overheads with [1] and the results are > before: avg of 30 results (total tests 50) : 102.96666666666667ms > after: avg of 30 results (total tests 50) : 113.16666666666667ms I assume that is with opt builds. Surprisingly high overhead, given that there is the NS_TARGET_CHAIN_WANTS_PRE_HANDLE_EVENT optimization. I wonder if we could improve the performance somehow. Have you perhaps profiled the builds? I don't think Gecko Profiler gives good enough data for this kind of case. I use Zoom on linux. I think Instruments works well on OSX.
Attachment #8805405 - Flags: review?(bugs) → review+
Comment on attachment 8805406 [details] [diff] [review] Part2: Add nsIDOMEventTarget::PreHandleEvent >+nsresult >+nsINode::PreHandleEvent(const EventChainPreVisitor& aVisitor) >+{ >+ NS_ABORT(); >+ return NS_ERROR_NOT_IMPLEMENTED; >+} Hmm, why NS_ABORT and NS_ERROR_NOT_IMPLEMENTED? I would expect subclasses to call this. Or, perhaps there shouldn't be this at all, but just dummy implementation in nsIDOMEventTarget.idl, using %{C++ stuff there. >- // Event target chain is created. Handle the chain. >+ // Event target chain is created. PreHandle the chain. >+ for (uint32_t i = 0; i < chain.Length(); ++i) { >+ if (chain[i].WantsPreHandleEvent()) { >+ chain[i].PreHandleEvent(preVisitor); >+ } Since this is rather hot path, I actually wonder if WantsPreHandleEvent() could be called inside PreHandleEvent and if WantsPreHandleEvent() returns false, return early from the method. That should optimize out one chain[i] access. (Perhaps compiler optimizes that out already, but is it guaranteed on all the platforms?) >+++ b/dom/interfaces/events/nsIDOMEventTarget.idl >@@ -222,16 +222,24 @@ interface nsIDOMEventTarget : nsISupport > * event target chain for event dispatching. > * > * @note Only EventDispatcher should call this method. > */ > [noscript, nostdcall] > void GetEventTargetParent(in EventChainPreVisitorRef aVisitor); > > /** >+ * Called before the capture phase of the event flow and after event target >+ * chain creation. This is used to handle those stuffs must be executed before >+ * dispatch event to DOM >+ */ >+ [noscript, nostdcall] >+ void PreHandleEvent([const] in EventChainPreVisitorRef aVisitor); So this could become something like %{C++ virtual nsresult PreHandleEvent(EventChainPreVisitor& aVisitor) { return NS_OK; } %} Those fixed, r+
Attachment #8805406 - Flags: review?(bugs) → review+
Comment on attachment 8805407 [details] [diff] [review] Part3: Add EventTargetChainItem::GetFirstEventTarget ># HG changeset patch ># User Stone Shih <sshih@mozilla.com> ># Date 1477099300 -28800 ># å­ 10æ 22 09:21:40 2016 +0800 ># Node ID ef14c598deb8683afc485278e1305e61295d0c77 ># Parent c4e06d027eaba780eb2570691bd886ca221e4e92 >Bug 1305458 Part3: Add EventTargetChainItem::GetFirstEventTarget > >MozReview-Commit-ID: EssfUw8QQqH > >diff --git a/dom/events/EventDispatcher.cpp b/dom/events/EventDispatcher.cpp >--- a/dom/events/EventDispatcher.cpp >+++ b/dom/events/EventDispatcher.cpp >@@ -161,16 +161,27 @@ public: > static void DestroyLast(nsTArray<EventTargetChainItem>& aChain, > EventTargetChainItem* aItem) > { > uint32_t lastIndex = aChain.Length() - 1; > MOZ_ASSERT(&aChain[lastIndex] == aItem); > aChain.RemoveElementAt(lastIndex); > } > >+ >+ static int GetFirstEventTargetIdx(nsTArray<EventTargetChainItem>& aChain) Not sure how useful this method is. Could you just define some static const uint32_t kFirstEventTargetIdx = 0; And then GetFirstEventTarget could be static EventTargetChainItem* GetFirstEventTarget( nsTArray<EventTargetChainItem>& aChain) { return &aChain[kFirstEventTargetIdx]; > uint32_t chainLength = aChain.Length(); >+ uint32_t firstEventTargetIdx = >+ EventTargetChainItem::GetFirstEventTargetIdx(aChain); Then you'd not need this, since kFirstEventTargetIdx could be used everywhere. >+ MOZ_ASSERT(EventTargetChainItem::GetFirstEventTargetIdx(chain) == 0); This could be then removed Those fixed, r+
Attachment #8805407 - Flags: review?(bugs) → review+
Attachment #8805409 - Flags: review?(bugs) → review+
Attachment #8805410 - Flags: review?(bugs) → review+
Attachment #8805411 - Flags: review?(bugs) → review+
Comment on attachment 8805412 [details] [diff] [review] Part8: Move dispatch XUL command from nsXULElement::GetEventTargetParent to PreHandleEvent Good that mItemFlags gives this flexibility.
Attachment #8805412 - Flags: review?(bugs) → review+
Comment on attachment 8805406 [details] [diff] [review] Part2: Add nsIDOMEventTarget::PreHandleEvent Oh, wait, why is EventTargetChainItem::PreHandleEvent right? Nothing sets the right mItemFlags or mItemData.
Attachment #8805406 - Flags: review+ → review-
Comment on attachment 8805406 [details] [diff] [review] Part2: Add nsIDOMEventTarget::PreHandleEvent Actually, I think PreHandleEvent shouldn't take EventChainPreVisitor& as param, since some of the properties it has aren't valid when PreHandleEvent is called. Perhaps better to pass EventChainVisitor&.
Comment on attachment 8805413 [details] [diff] [review] Part9: Move fire events and set value from HTMLInputElement::GetEventTargetParent to PreHandleEvent >+HTMLInputElement::PreHandleEvent(const EventChainPreVisitor& aVisitor) >+{ >+ if (!aVisitor.mPresContext) { >+ return nsGenericHTMLElement::PreHandleEvent(aVisitor); >+ } Should use nsGenericHTMLFormElementWithState here. The existing code has a bug. But can be fixed also in a followup.
Attachment #8805413 - Flags: review?(bugs) → review+
Comment on attachment 8805486 [details] [diff] [review] Part4: Call EventTargetChainItem::PreHandleEvent even it sets mCanHandle=false >+EventTargetChainItem::PreHandleEvent(EventChainPreVisitor& aVisitor) > { >+ aVisitor.Reset(); >+ aVisitor.mItemFlags = mItemFlags; >+ aVisitor.mItemData = mItemData; > Unused << mTarget->PreHandleEvent(aVisitor); > } Ah, this is fixed here. nm my earlier comment then > // Capture > aVisitor.mEvent->mFlags.mInCapturePhase = true; > aVisitor.mEvent->mFlags.mInBubblingPhase = false; > for (uint32_t i = chainLength - 1; i > firstEventTargetIdx; --i) { > EventTargetChainItem& item = aChain[i]; >+ if (item.PreHandleEventOnly()) { >+ continue; >+ } > if ((!aVisitor.mEvent->mFlags.mNoContentDispatch || > item.ForceContentDispatch()) && > !aVisitor.mEvent->PropagationStopped()) { > item.HandleEvent(aVisitor, aCd); > } > > if (item.GetNewTarget()) { > // item is at anonymous boundary. Need to retarget for the child items. >@@ -423,16 +452,19 @@ EventTargetChainItem::HandleEventTargetC > if (aVisitor.mEvent->mFlags.mInSystemGroup) { > targetItem.PostHandleEvent(aVisitor); > } > > // Bubble > aVisitor.mEvent->mFlags.mInCapturePhase = false; > for (uint32_t i = firstEventTargetIdx + 1; i < chainLength; ++i) { > EventTargetChainItem& item = aChain[i]; >+ if (item.PreHandleEventOnly()) { >+ continue; >+ } Why the target phase doesn't need to check PreHandleEventOnly()? Though, that wouldn't be quite right either. Basically I don't understand why firstEventTargetIdx is right in the loops. What if both 0 and 1 indexes set PreHandleEventOnly()? Shouldn't GetFirstEventTargetIdx loop from chain until it finds one entry which returns false for PreHandleEventOnly(). (or nm my earlier comments about sFirstEventTargetIdx)
Attachment #8805486 - Flags: review?(bugs) → review-
Attachment #8805414 - Flags: review?(bugs) → review+
Comment on attachment 8805406 [details] [diff] [review] Part2: Add nsIDOMEventTarget::PreHandleEvent ok, this is fine since mItemFlags or mItemData are set in a different patch. But still I think PreHandleEvent shouldn't take EventChainPreVisitor& as param, if possible, since some of the properties it has aren't valid when PreHandleEvent is called. Perhaps better to pass EventChainVisitor&. That fixed, or explained why it can't be done, r+
Attachment #8805406 - Flags: review- → review+
(In reply to Olli Pettay [:smaug] from comment #30) > Comment on attachment 8805486 [details] [diff] [review] > Part4: Call EventTargetChainItem::PreHandleEvent even it sets > mCanHandle=false > Why the target phase doesn't need to check PreHandleEventOnly()? Though, > that wouldn't be quite right either. It's because I thought the target node must be the node that set EventChainPreVisitor::mCanHandle=true. Is my understanding correct? > Basically I don't understand why firstEventTargetIdx is right in the loops. > What if both 0 and 1 indexes set PreHandleEventOnly()? > Shouldn't GetFirstEventTargetIdx loop from chain until it finds one entry > which returns false for PreHandleEventOnly(). In current implementation, if the hit node set EventChainPreVisitor::mCanHandle=false, we try to propagate event to chrome when EventChainPreVisitor::mAutomaticChromeDispatch=true. If the chrome target still can not handle the event, then we won't continue to dispatch the event. When dispatching events to DOM, either the original target node or the chrome target can handle the event. So there should be up to 1 node that is PreHandleEventOnly in the chain. But it's less readability so I'd refine the patch to use a loop to find the target entry.
(In reply to Ming-Chou Shih [:stone] from comment #32) > refine the patch to use a loop to find the target entry. Maybe I should use sFirstEventTargetIdx with less overhead
Ming-Chou Shih [:stone] from comment #33) > Maybe I should use sFirstEventTargetIdx with less overhead Nah, that comment was for previous patches before I saw how you're going to use the index. (In reply to Ming-Chou Shih [:stone] from comment #32) > In current implementation, if the hit node set > EventChainPreVisitor::mCanHandle=false, we try to propagate event to chrome > when EventChainPreVisitor::mAutomaticChromeDispatch=true. If the chrome > target still can not handle the event, then we won't continue to dispatch > the event. When dispatching events to DOM, either the original target node > or the chrome target can handle the event. So there should be up to 1 node > that is PreHandleEventOnly in the chain. But it's less readability so I'd > refine the patch to use a loop to find the target entry. But it may not be the first (non-chrome) item in the chain which sets PreHandleEventOnly and mCanHandle=false
Refine GetFirstEventTargetIdx to loop from chain until it finds the first entry which is not PreHandleEventOnly
Attachment #8805486 - Attachment is obsolete: true
Tested with patch 1~11 avg of 30 results (total tests 50) : 111.6ms avg of 30 results (total tests 50) : 111.4ms avg of 30 results (total tests 50) : 111.96666666666667ms avg of 30 results (total tests 50) : 112ms avg of 30 results (total tests 50) : 112.23333333333333ms Tested with patch 1~12 avg of 30 results (total tests 50) : 109.33333333333333ms avg of 30 results (total tests 50) : 110.03333333333333ms avg of 30 results (total tests 50) : 109.56666666666666ms avg of 30 results (total tests 50) : 109.43333333333334ms avg of 30 results (total tests 50) : 110.3ms Tested with patch 1~13 avg of 30 results (total tests 50) : 110.2ms avg of 30 results (total tests 50) : 109.76666666666667ms avg of 30 results (total tests 50) : 109.16666666666667ms avg of 30 results (total tests 50) : 109.56666666666666ms avg of 30 results (total tests 50) : 109.63333333333334ms
Profiling with zoom on Linux, looks like the extra overheads are induced by accessing to the chain, invocation of PreHandleEvent and PreHandleEventOnly.
Attachment #8806669 - Flags: review?(bugs)
Refined patch to use inline functions and explicit constructor.
Attachment #8806668 - Attachment is obsolete: true
Attachment #8806968 - Flags: review?(bugs)
Attachment #8806665 - Flags: review?(bugs)
Refine the logic of calling PreHandleEvent when the original target and chrome target set mCanHandle=false
Attachment #8806663 - Attachment is obsolete: true
Attachment #8806997 - Flags: review?(bugs)
Fix typo
Attachment #8806968 - Attachment is obsolete: true
Attachment #8806968 - Flags: review?(bugs)
Attachment #8806998 - Flags: review?(bugs)
Attachment #8807037 - Attachment is obsolete: true
Attachment #8807037 - Flags: review?(bugs)
Attachment #8806665 - Flags: review?(bugs) → review+
Comment on attachment 8806669 [details] [diff] [review] Part13: Define functions access to WidgetEvent::mFlags as inline functions This isn't need, based on some comments I found. Apparently "all member functions defined inside class definition are inline"
Attachment #8806669 - Flags: review?(bugs)
Comment on attachment 8806998 [details] [diff] [review] Part12: Refine EventTargetChain flags to reduce overheads Drop 'inline' everywhere. s/wihch/which/ I think having the separate flags in EventTargetChainItem would have been a bit nicer, but this is ok too.
Attachment #8806998 - Flags: review?(bugs) → review+
Comment on attachment 8806997 [details] [diff] [review] Part4: Call EventTargetChainItem::PreHandleEvent even it sets mCanHandle=false >- static uint32_t GetFirstEventTargetIdx(nsTArray<EventTargetChainItem>& aChain) >+ static uint32_t GetFirstCanHandleEventTargetIdx(nsTArray<EventTargetChainItem>& aChain) > { >+ // aChain[i].PreHandleEventOnly() = true only when the original target Why 'original target'? Shouldn't it be just 'target'. >+ // element wants PreHandleEvent and set mCanHandle=false. So we find the >+ // first element which can handle the event. >+ for (uint32_t i = 0; i < aChain.Length(); ++i) { >+ if (!aChain[i].PreHandleEventOnly()) { So shouldn't mCanHandle be true here. Could we assert that.
Attachment #8806997 - Flags: review?(bugs) → review+
Comment on attachment 8807037 [details] [diff] [review] Part14: Refine event target chain creation oh, this was still in my queue, but marked as obsolete.
Attachment #8807037 - Flags: review?(bugs)
(In reply to Olli Pettay [:smaug] from comment #47) > Comment on attachment 8806997 [details] [diff] [review] > Part4: Call EventTargetChainItem::PreHandleEvent even it sets > mCanHandle=false > >+ // element wants PreHandleEvent and set mCanHandle=false. So we find the > >+ // first element which can handle the event. > >+ for (uint32_t i = 0; i < aChain.Length(); ++i) { > >+ if (!aChain[i].PreHandleEventOnly()) { > So shouldn't mCanHandle be true here. Could we assert that. We don't cache mCanHandle in EventTargetChainItem. Should I add one more bool flag to keep it?
Attachment #8807087 - Flags: review?(bugs)
Attachment #8807087 - Flags: review?(bugs) → review+
Attachment #8805405 - Attachment is obsolete: true
Attachment #8805406 - Attachment is obsolete: true
Attachment #8805407 - Attachment is obsolete: true
Attachment #8805409 - Attachment is obsolete: true
Attachment #8805410 - Attachment is obsolete: true
Attachment #8805411 - Attachment is obsolete: true
Attachment #8805412 - Attachment is obsolete: true
Attachment #8805413 - Attachment is obsolete: true
Attachment #8805414 - Attachment is obsolete: true
Attachment #8806665 - Attachment is obsolete: true
Attachment #8806669 - Attachment is obsolete: true
Attachment #8806997 - Attachment is obsolete: true
Attachment #8806998 - Attachment is obsolete: true
Attachment #8807087 - Attachment is obsolete: true
Attachment #8808866 - Flags: review+
Attached patch Part10: Add test case. r=smaug (obsolete) — — Splinter Review
Attachment #8808875 - Flags: review+
Comment on attachment 8810758 [details] [diff] [review] Bug 1305458 Part14: Refine assertion in EventTargetChainItem::Create EventTargetChainItem::Create will check if the child item is the last item in the array. However, the patches of this bug may keep some items which are PreHandleEventOnly in the array so I have to refine the assertion.
Attachment #8810758 - Flags: review?(bugs)
Comment on attachment 8810758 [details] [diff] [review] Bug 1305458 Part14: Refine assertion in EventTargetChainItem::Create s/Fine/Find/
Attachment #8810758 - Flags: review?(bugs) → review+
Rebased
Attachment #8808867 - Attachment is obsolete: true
Attachment #8811658 - Flags: review+
Rebased
Attachment #8808868 - Attachment is obsolete: true
Attachment #8811659 - Flags: review+
Rebased
Attachment #8808872 - Attachment is obsolete: true
Attachment #8811663 - Flags: review+
Rebased
Attachment #8808875 - Attachment is obsolete: true
Attachment #8811666 - Flags: review+
Rebased
Attachment #8808878 - Attachment is obsolete: true
Attachment #8811669 - Flags: review+
Keywords: checkin-needed
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/fa8842555071 Part1: Rename nsIDOMEventTarget::PreHandleEvent to nsIDOMEventTarget::GetEventTargetParent. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/d6c53113dd77 Part2: Add nsIDOMEventTarget::PreHandleEvent. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/ec4c31d96593 Part3: Add EventTargetChainItem::GetFirstEventTarget. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/fdf729308c40 Part4: Call EventTargetChainItem::PreHandleEvent even it sets mCanHandle=false. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/76bc134bda00 Part5: Move form control frame focus/blur from nsGenericHTMLFormElement::GetEventTargetParent to PreHandleEvent. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/5d88da6f4b03 Part6: Move fire change event from HTMLTextAreaElement::GetEventTargetParent to PreHandleEvent. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/be4e001288d0 Part7: Refine nsXULElement::GetEventTargetParent. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/1a75a459c545 Part8: Move dispatch XUL command from nsXULElement::GetEventTargetParent to PreHandleEvent. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/5fbc6dab40e3 Part9: Move fire events and set value from HTMLInputElement::GetEventTargetParent to PreHandleEvent. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/5db030d2903a Part10: Add test case. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/f52950131023 Part11: Let HTMLInputElement delegate event handling to it's parent class. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/a961e6f68831 Part12: Refine EventTargetChain flags to reduce overheads. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/2078069f8c16 Part13: Refine event target chain creation. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/fbab4273091a Part14: Refine assertion in EventTargetChainItem::Create r=smaug
Keywords: checkin-needed
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: