Closed
Bug 864206
Opened 9 years ago
Closed 9 years ago
Use dom::Touch instead of nsIDOMTouch where possible
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: dzbarsky, Assigned: dzbarsky)
Details
Attachments
(2 files)
49.19 KB,
patch
|
Ms2ger
:
review+
|
Details | Diff | Splinter Review |
1.86 KB,
patch
|
Ms2ger
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Attachment #740139 -
Flags: review?(Ms2ger)
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #740149 -
Flags: review?(Ms2ger)
Comment 2•9 years ago
|
||
You must update uuid of nsIDOMTouch
Comment 3•9 years ago
|
||
Comment on attachment 740139 [details] [diff] [review] Patch Review of attachment 740139 [details] [diff] [review]: ----------------------------------------------------------------- What made the extra includes necessary? ::: content/events/src/nsDOMTouchEvent.cpp @@ +16,5 @@ > using namespace mozilla; > using namespace mozilla::dom; > > // TouchList > +nsDOMTouchList::nsDOMTouchList(nsTArray<nsRefPtr<Touch> > &aTouches) & to the left @@ +56,5 @@ > *aRetVal = nullptr; > for (uint32_t i = 0; i < mPoints.Length(); ++i) { > + nsRefPtr<Touch> point = mPoints[i]; > + if (point && aIdentifier == point->Identifier()) { > + *aRetVal = point.forget().get(); point.forget(aRetVal); ::: js/xpconnect/src/dom_quickstubs.qsconf @@ -149,5 @@ > 'nsIDOMBlob': 'nsIDOMFile', > > 'nsIIndexedDatabaseUsageCallback': 'nsIIndexedDatabaseManager', > > - 'nsIDOMTouch': 'nsIDOMTouchEvent', Last time I tried this, the nsIDOMTouchList quickstubs were unhappy with it. ::: layout/base/nsPresShell.cpp @@ +5899,5 @@ > > static PLDHashOperator > +AppendToTouchList(const uint32_t& aKey, nsRefPtr<Touch>& aData, void *aTouchList) > +{ > + nsTArray<nsRefPtr<Touch> > *touches = static_cast<nsTArray<nsRefPtr<Touch> > *>(aTouchList); * to the left @@ +6285,2 @@ > for (uint32_t i = 0; i < touches.Length(); ++i) { > + Touch *touch = touches[i]; Ditto @@ +6298,2 @@ > nsCOMPtr<nsIDOMEventTarget> targetPtr; > oldTouch->GetTarget(getter_AddRefs(targetPtr)); Fix this too? @@ +6673,5 @@ > case NS_TOUCH_END: { > // Remove the changed touches > // need to make sure we only remove touches that are ending here > nsTouchEvent* touchEvent = static_cast<nsTouchEvent*>(aEvent); > + nsTArray<nsRefPtr<Touch> > &touches = touchEvent->touches; & to the left, and fix the double space @@ +6678,2 @@ > for (uint32_t i = 0; i < touches.Length(); ++i) { > + Touch *touch = touches[i]; * to the left @@ +6706,4 @@ > bool haveChanged = false; > for (int32_t i = touches.Length(); i; ) { > --i; > + Touch *touch = touches[i]; Ditto @@ +6718,5 @@ > if (!oldTouch) { > touches.RemoveElementAt(i); > continue; > } > + if(touch->Equals(oldTouch)) { 'if (' @@ +6870,5 @@ > nsTouchEvent* touchEvent = static_cast<nsTouchEvent*>(aEvent); > > // loop over all touches and dispatch events on any that have changed > for (uint32_t i = 0; i < touchEvent->touches.Length(); ++i) { > + Touch *touch = touchEvent->touches[i]; * ::: widget/windows/winrt/MetroInput.h @@ +27,5 @@ > +namespace mozilla { > + namespace dom { > + class Touch; > + } > +} No need for the indentation, even if the following code is wrong @@ +235,5 @@ > nsTouchEvent mTouchEvent; > void DispatchPendingTouchEvent(); > void DispatchPendingTouchEvent(nsEventStatus& status); > nsBaseHashtable<nsUint32HashKey, > + nsCOMPtr<mozilla::dom::Touch>, nsCOMPtr?
Attachment #740139 -
Flags: review?(Ms2ger) → review+
Comment 4•9 years ago
|
||
Comment on attachment 740149 [details] [diff] [review] Move variables from nsIDOMTouch to Touch Review of attachment 740149 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/events/src/Touch.h @@ +116,3 @@ > bool mPointsInitialized; > +public: > + bool mChanged; Please tell me people don't rely on these being public...
Attachment #740149 -
Flags: review?(Ms2ger) → review+
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to :Ms2ger from comment #3) > Comment on attachment 740139 [details] [diff] [review] > Patch > > Review of attachment 740139 [details] [diff] [review]: > ----------------------------------------------------------------- > > What made the extra includes necessary? > Not including nsDOMUIEvent.h in Touch.h (In reply to :Ms2ger from comment #4) > Comment on attachment 740149 [details] [diff] [review] > Move variables from nsIDOMTouch to Touch > > Review of attachment 740149 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: content/events/src/Touch.h > @@ +116,3 @@ > > bool mPointsInitialized; > > +public: > > + bool mChanged; > > Please tell me people don't rely on these being public... Yup...
Assignee | ||
Comment 6•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/43872f1bfbda
Whiteboard: [leave open]
Assignee | ||
Comment 8•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/43c0c4dd5a5c
Assignee | ||
Updated•9 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
Updated•3 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•