Closed
Bug 1094913
Opened 11 years ago
Closed 11 years ago
Gotpointercapture is missing pointerType attribute
Categories
(Core :: DOM: Events, defect)
Core
DOM: Events
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: evgeny.agafonchikov, Assigned: alessarik)
References
Details
(Keywords: testcase)
Attachments
(2 files, 2 obsolete files)
19.04 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
9.11 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
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.
Updated•11 years ago
|
Assignee | ||
Comment 1•11 years ago
|
||
Tests for check pointerType attribute on got/lostpointercapture events
Attachment #8519920 -
Flags: review?(bugs)
Attachment #8519920 -
Flags: feedback?(mbrubeck)
Assignee | ||
Comment 2•11 years ago
|
||
+ 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)
Assignee | ||
Comment 3•11 years ago
|
||
Patch for resolving issue
Attachment #8519925 -
Flags: review?(bugs)
Attachment #8519925 -
Flags: feedback?(mbrubeck)
Assignee | ||
Comment 4•11 years ago
|
||
Tests with patch: https://tbpl.mozilla.org/?tree=Try&rev=a19cd781373c
Assignee | ||
Comment 5•11 years ago
|
||
Test without patch: https://tbpl.mozilla.org/?tree=Try&rev=08913f4b0462
Test with patch: https://tbpl.mozilla.org/?tree=Try&rev=42a83eedf432
Comment 6•11 years ago
|
||
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 7•11 years ago
|
||
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+
Assignee | ||
Comment 8•11 years ago
|
||
+ 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)
Assignee | ||
Comment 9•11 years ago
|
||
(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.
Updated•11 years ago
|
Attachment #8520697 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 10•11 years ago
|
||
Test without patch: https://tbpl.mozilla.org/?tree=Try&rev=29f2cf20b1f3
Test with patch: https://tbpl.mozilla.org/?tree=Try&rev=cf197d4088e1
Assignee | ||
Comment 11•11 years ago
|
||
If nobody have objections, please, anybody set 'checkin' flag. I have no rights for doing this.
Updated•11 years ago
|
Comment 12•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6c474c0cdf9
https://hg.mozilla.org/integration/mozilla-inbound/rev/d8e5947b82cb
Assignee: nobody → alessarik
Keywords: checkin-needed
Assignee | ||
Updated•11 years ago
|
OS: Windows 8.1 → All
Hardware: x86_64 → All
https://hg.mozilla.org/mozilla-central/rev/d6c474c0cdf9
https://hg.mozilla.org/mozilla-central/rev/d8e5947b82cb
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Updated•11 years ago
|
Attachment #8520697 -
Flags: feedback?(mbrubeck)
Updated•11 years ago
|
Attachment #8519924 -
Flags: feedback?(mbrubeck)
You need to log in
before you can comment on or make changes to this bug.
Description
•