Closed Bug 1094913 Opened 7 years ago Closed 7 years ago

Gotpointercapture is missing pointerType attribute

Categories

(Core :: DOM: Events, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: evgeny.agafonchikov, Assigned: alessarik)

References

Details

(Keywords: testcase)

Attachments

(2 files, 2 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0
Build ID: 20140923175406

Steps to reproduce:

1. Open http://www.w3c-test.org/pointerevents/pointerevent_releasepointercapture_events_to_original_target-manual.html in FF with Modern UI support (any of Modern UI or Desktop modes)
2. Click black rectangle


Actual results:

String "gotpointercapture properties for pointerType = mouse" is not displayed in the results.
event.pointerType is "" for gotpointercapture


Expected results:

Presence of "gotpointercapture properties for pointerType = mouse" string in the results.
event.pointerType should be "mouse" for gotpointercapture in this case.
Component: Untriaged → DOM: Events
Keywords: testcase
Product: Firefox → Core
Tests for check pointerType attribute on got/lostpointercapture events
Attachment #8519920 - Flags: review?(bugs)
Attachment #8519920 - Flags: feedback?(mbrubeck)
+ Small changes
Attachment #8519920 - Attachment is obsolete: true
Attachment #8519920 - Flags: review?(bugs)
Attachment #8519920 - Flags: feedback?(mbrubeck)
Attachment #8519924 - Flags: review?(bugs)
Attachment #8519924 - Flags: feedback?(mbrubeck)
Attached patch gotlostpc_events_type_ver1.diff (obsolete) — Splinter Review
Patch for resolving issue
Attachment #8519925 - Flags: review?(bugs)
Attachment #8519925 - Flags: feedback?(mbrubeck)
Blocks: 1042108
Comment on attachment 8519925 [details] [diff] [review]
gotlostpc_events_type_ver1.diff

>+ConvertPointerTypeToString(nsAString& aPointerTypeDest, uint16_t aPointerTypeSrc)
Please swap the order of the parameters. "outparam" should be the latter one.

>+nsIPresShell::GetPointerType(uint32_t aPointerId)
>+{
>+  PointerInfo* pointerInfo = nullptr;
>+  if (gActivePointersIds->Get(aPointerId, &pointerInfo) && pointerInfo)
>+    return pointerInfo->mPointerType;
Always use {} with 'if'
Attachment #8519925 - Flags: review?(bugs) → review+
Comment on attachment 8519924 [details] [diff] [review]
gotlostpc_events_type_test_ver1.diff

>-      parent.is(test_listener,      false,  "listener shouldn't receive any events");
>+      parent.is(test_listener,      false,  "listener should not receive any events");
...

>     function finishTest() {
>-      setTimeout(function() {
>-        parent.is(test_down_got,  true,  "Part 2: pointerdown event should be received by target");
>-        parent.is(test_listener,  false, "Part 2: listener shouldn't receive any events");
>-        logger("finishTest");
>-        parent.finishTest();
>-      }, 1000);
>+      parent.is(test_down_got,  true,  "Part 2: pointerdown event should be received by target");
>+      parent.is(test_listener,  false, "Part 2: listener should not receive any events");
>+      logger("finishTest");
>+      parent.finishTest();
etc.

In general, please don't make unrelated changes. A patch should implement/fix one issue. Not several
totally unrelated ones. So in the future try to do clean ups in separate patches please.

But, ok, this should be fine.
Attachment #8519924 - Flags: review?(bugs) → review+
+ Changes: Swap order of arguments
+ Changes: Add {} to if
- Changes: Remove build issue
Attachment #8519925 - Attachment is obsolete: true
Attachment #8519925 - Flags: feedback?(mbrubeck)
Attachment #8520697 - Flags: review?(bugs)
Attachment #8520697 - Flags: feedback?(mbrubeck)
(In reply to Olli Pettay [:smaug] from comment #7)
> In general, please don't make unrelated changes. A patch should
> implement/fix one issue. Not several totally unrelated ones.
> So in the future try to do clean ups in separate patches please.
I only wanted to decrease time of current test.
Attachment #8520697 - Flags: review?(bugs) → review+
If nobody have objections, please, anybody set 'checkin' flag. I have no rights for doing this.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: checkin-needed
OS: Windows 8.1 → All
Hardware: x86_64 → All
Attachment #8520697 - Flags: feedback?(mbrubeck)
Attachment #8519924 - Flags: feedback?(mbrubeck)
You need to log in before you can comment on or make changes to this bug.