Closed Bug 1080360 Opened 7 years ago Closed 7 years ago

The lostpointercapture event on document when capturing element is removed

Categories

(Core :: DOM: Events, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: alessarik, Assigned: alessarik)

References

Details

Attachments

(2 files, 3 obsolete files)

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

Steps to reproduce:

Test assertion 11.3



Expected results:

If the pointer capture target override is removed from the document tree, clear the pending pointer capture target override and pointer capture target override nodes and fire a Pointer Event named lostpointercapture at the document.
Component: Untriaged → DOM: Events
Product: Firefox → Core
Blocks: 1042108
OS: Windows 8 → All
Hardware: x86_64 → All
Attached patch lostpc_on_document_ver1.diff (obsolete) — Splinter Review
Add checking when lostpointercapture event should be fired at the Document
Attachment #8504031 - Flags: review?(bugs)
Attachment #8504031 - Flags: feedback?(mbrubeck)
Comment on attachment 8504031 [details] [diff] [review]
lostpc_on_document_ver1.diff

Why is this enough? The spec says we should clear the override states and 
dispatch the lostpointercapture event when an element is removed from document.
Couldn't we handle that case in EventStateManager::ContentRemoved?

We need some test here too.
Attachment #8504031 - Flags: review?(bugs) → review-
(In reply to Olli Pettay [:smaug] from comment #2)
> Why is this enough? The spec says we should clear the override states and 
> dispatch the lostpointercapture event when an element is removed from document.
Specification says:
> If the pointer capture target override is removed from the document tree, ...
But I cannot see something like "When element is removed...".
I think that specification have no any pointer to time of firing lostpointercapture event.
"If the pointer capture target override is removed from the document tree, clear the pending pointer capture target override and pointer capture target override nodes and fire a Pointer Event named lostpointercapture at the document."
Attached patch lostpc_on_document_ver2.diff (obsolete) — Splinter Review
+ Changes: Explicity release pointer capture from removing content

(In reply to Olli Pettay [:smaug] from comment #2)
> Couldn't we handle that case in EventStateManager::ContentRemoved?
I think nsPresShell::ContentRemoved is more likely place for it
Attachment #8504031 - Attachment is obsolete: true
Attachment #8504031 - Flags: feedback?(mbrubeck)
Attachment #8506031 - Flags: review?(bugs)
Attachment #8506031 - Flags: feedback?(mbrubeck)
+ Test for checking lostpointercapture event on document
Attachment #8506098 - Flags: review?(bugs)
Attachment #8506098 - Flags: feedback?(mbrubeck)
Attachment #8506098 - Flags: review?(bugs) → review+
Comment on attachment 8506031 [details] [diff] [review]
lostpc_on_document_ver2.diff

>+  // We should check that aChild has not contain pointer capturing elements.
s/has not/does not/


>+  // In other case we should release pointer capture for this elements.
If it does we should release the pointer capture for the elements.


something like that.
Attachment #8506031 - Flags: review?(bugs) → review+
Attached patch lostpc_on_document_ver3.diff (obsolete) — Splinter Review
+ Changes comment according with comments
Attachment #8506031 - Attachment is obsolete: true
Attachment #8506031 - Flags: feedback?(mbrubeck)
Attachment #8506723 - Flags: review?(bugs)
Attachment #8506723 - Flags: feedback?(mbrubeck)
Comment on attachment 8506723 [details] [diff] [review]
lostpc_on_document_ver3.diff

>+      if (nsIDocument* doc = aCaptureTarget->OwnerDoc()) {
>+        doc->DispatchEvent(event->InternalDOMEvent(), &dummy);
OwnerDoc() returns always a valid value.
So,
aCaptureTarget->OwnerDoc()->DispatchEvent(event->InternalDOMEvent(), &dummy);


r+
Attachment #8506723 - Flags: review?(bugs) → review+
+ Changes according with comments
Attachment #8506723 - Attachment is obsolete: true
Attachment #8506723 - Flags: feedback?(mbrubeck)
Attachment #8507724 - Flags: review?(bugs)
Attachment #8507724 - Flags: feedback?(mbrubeck)
Attachment #8507724 - Flags: review?(bugs) → review+
If nobody have objections, I put checkin flag.
Keywords: checkin-needed
Attachment #8506098 - Flags: feedback?(mbrubeck) → feedback+
Attachment #8507724 - Flags: feedback?(mbrubeck) → feedback+
https://hg.mozilla.org/mozilla-central/rev/21e932e78181
https://hg.mozilla.org/mozilla-central/rev/f45c1a2f0177
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.