117620 2013-06-13 17:58:21 -0700 Clicking on snapshotting plug-ins does not restart them 2013-06-14 10:10:50 -0700 1 1 1 Unclassified WebKit Plug-ins 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 dino dino commit-queue esprehn+autocc thorton oldest_to_newest 900343 0 dino 2013-06-13 17:58:21 -0700 If a plug-in is snapshotting and the user clicks on it, it should immediately restart. <rdar://problem/13821729> 900344 1 204657 dino 2013-06-13 18:01:49 -0700 Created attachment 204657 Patch 900345 2 204657 dino 2013-06-13 18:05:58 -0700 Comment on attachment 204657 Patch Ignore this. I tried to sneak it by but it is bad coding. 900346 3 204657 darin 2013-06-13 18:08:41 -0700 Comment on attachment 204657 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204657&action=review > Source/WebCore/html/HTMLPlugInElement.cpp:223 > + if (displayState() == WaitingForSnapshot && event->type() == eventNames().clickEvent && isPlugInImageElement()) { > + MouseEvent* mouseEvent = static_cast<MouseEvent*>(event); > + if (mouseEvent->button() == LeftButton) { > + toHTMLPlugInImageElement(this)->userDidClickSnapshot(mouseEvent, false); > + event->setDefaultHandled(); > + return; > + } > + } defaultEventHandler is a virtual function. Why not put this code in the HTMLPlugInImageElement class in the first place instead of down casting? 900348 4 204658 dino 2013-06-13 18:15:25 -0700 Created attachment 204658 Patch 900349 5 dino 2013-06-13 18:17:59 -0700 (In reply to comment #3) > (From update of attachment 204657 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=204657&action=review > > > Source/WebCore/html/HTMLPlugInElement.cpp:223 > > + if (displayState() == WaitingForSnapshot && event->type() == eventNames().clickEvent && isPlugInImageElement()) { > > + MouseEvent* mouseEvent = static_cast<MouseEvent*>(event); > > + if (mouseEvent->button() == LeftButton) { > > + toHTMLPlugInImageElement(this)->userDidClickSnapshot(mouseEvent, false); > > + event->setDefaultHandled(); > > + return; > > + } > > + } > > defaultEventHandler is a virtual function. Why not put this code in the HTMLPlugInImageElement class in the first place instead of down casting? Yeah... that's what I ended up doing. I'll even admit I thought of doing it up front :( 900350 6 204658 simon.fraser 2013-06-13 18:18:15 -0700 Comment on attachment 204658 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204658&action=review > Source/WebCore/html/HTMLPlugInImageElement.cpp:716 > + if (displayState() == WaitingForSnapshot && event->type() == eventNames().clickEvent && isPlugInImageElement()) { I think this would be slightly clearer if you did the event->type() last. 900352 7 dino 2013-06-13 18:21:05 -0700 Committed r151576: <http://trac.webkit.org/changeset/151576> 900353 8 204658 darin 2013-06-13 18:21:28 -0700 Comment on attachment 204658 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204658&action=review > Source/WebCore/html/HTMLPlugInElement.h:83 > + virtual void defaultEventHandler(Event*); Should be protected, not public. And as long as you are touching it, should add OVERRIDE. > Source/WebCore/html/HTMLPlugInImageElement.h:128 > + virtual void defaultEventHandler(Event*); Should add OVERRIDE. Could be private instead of protected. 900597 9 dino 2013-06-14 10:10:50 -0700 Follow-up commit for Darin's comments: https://trac.webkit.org/r151599 204657 2013-06-13 18:01:49 -0700 2013-06-13 18:15:23 -0700 Patch bug-117620-20130613180008.patch text/plain 2312 dino U3VidmVyc2lvbiBSZXZpc2lvbjogMTUxNTcyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNWY0MGQ3MTI5ODRjYjg5 OTBkMjlmMWE0Yjg5OTY2ZjcwODdkNzJkNi4uOWQ4MzAyYWIwMjA0Mjc5NGY3OWQ3ZmU4NzU5ZjAz MGEyNzMxMjEyOSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIyIEBACisyMDEzLTA2LTEzICBEZWFu IEphY2tzb24gIDxkaW5vQGFwcGxlLmNvbT4KKworICAgICAgICBDbGlja2luZyBvbiBzbmFwc2hv dHRpbmcgcGx1Zy1pbnMgZG9lcyBub3QgcmVzdGFydCB0aGVtCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMTc2MjAKKyAgICAgICAgPHJkYXI6Ly9wcm9i bGVtLzEzODIxNzI5PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIEhUTUxQbHVnSW5FbGVtZW50IGhhcyBhbiBldmVudCBoYW5kbGVyIHdoaWNoIHdvdWxk IHJldHVybgorICAgICAgICBpbW1lZGlhdGVseSBpZiB0aGUgZWxlbWVudCB3YXMgbm90IHNuYXBz aG90dGVkLCByZXN0YXJ0aW5nCisgICAgICAgIG9yIHBsYXlpbmcuIFRoZXJlIGlzIGEgY2FzZSBp dCBtaXNzZWQsIHdoaWNoIHdhcyBzbmFwc2hvdHRpbmcKKyAgICAgICAgKFdhaXRpbmdGb3JTbmFw c2hvdCB0byBiZSBwcmVjaXNlKS4gSWYgd2UgZ2V0IGEgY2xpY2sgaW4gdGhpcworICAgICAgICBz dGF0ZSB0aGVuIGl0IHNob3VsZCBpbW1lZGlhdGVseSByZXN0YXJ0LgorCisgICAgICAgICogaHRt bC9IVE1MUGx1Z0luRWxlbWVudC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpIVE1MUGx1Z0luRWxl bWVudDo6ZGVmYXVsdEV2ZW50SGFuZGxlcik6IElmIHdlIHNlZSBhIGNsaWNrCisgICAgICAgIGFu ZCB3ZSdyZSBhIHNuYXBzaG90dGluZyBwbHVnaW4sIHByZXRlbmQgdGhlIGNsaWNrIHdhcyBlbm91 Z2ggdG8KKyAgICAgICAgcmVzdGFydCB0aGUgcGx1Z2luLgorCiAyMDEzLTA2LTEzICBBbmRyZWFz IEtsaW5nICA8YWtsaW5nQGFwcGxlLmNvbT4KIAogICAgICAgICBNb3ZlIE5vZGU6OnNob3VsZFVz ZUlucHV0TWV0aG9kKCkgdG8gRWxlbWVudC4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2h0 bWwvSFRNTFBsdWdJbkVsZW1lbnQuY3BwIGIvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MUGx1Z0lu RWxlbWVudC5jcHAKaW5kZXggZjU1NTliNTg5NDc5NGZhMzIyNTJhNDBlNzhiNzljYzU3MmNkM2Qy Yi4uYzQ1MTFmMDFjMmMzZDBkY2M1ZjczYTQwOTNkZWMzZjRiYTQzZmU4MCAxMDA2NDQKLS0tIGEv U291cmNlL1dlYkNvcmUvaHRtbC9IVE1MUGx1Z0luRWxlbWVudC5jcHAKKysrIGIvU291cmNlL1dl YkNvcmUvaHRtbC9IVE1MUGx1Z0luRWxlbWVudC5jcHAKQEAgLTM1LDYgKzM1LDggQEAKICNpbmNs dWRlICJGcmFtZUxvYWRlci5oIgogI2luY2x1ZGUgIkZyYW1lVHJlZS5oIgogI2luY2x1ZGUgIkhU TUxOYW1lcy5oIgorI2luY2x1ZGUgIkhUTUxQbHVnSW5JbWFnZUVsZW1lbnQuaCIKKyNpbmNsdWRl ICJNb3VzZUV2ZW50LmgiCiAjaW5jbHVkZSAiUGFnZS5oIgogI2luY2x1ZGUgIlBsdWdpblZpZXdC YXNlLmgiCiAjaW5jbHVkZSAiUmVuZGVyRW1iZWRkZWRPYmplY3QuaCIKQEAgLTIxMSw2ICsyMTMs MTUgQEAgdm9pZCBIVE1MUGx1Z0luRWxlbWVudDo6ZGVmYXVsdEV2ZW50SGFuZGxlcihFdmVudCog ZXZlbnQpCiAgICAgICAgICAgICByZXR1cm47CiAgICAgICAgIH0KIAorICAgICAgICBpZiAoZGlz cGxheVN0YXRlKCkgPT0gV2FpdGluZ0ZvclNuYXBzaG90ICYmIGV2ZW50LT50eXBlKCkgPT0gZXZl bnROYW1lcygpLmNsaWNrRXZlbnQgJiYgaXNQbHVnSW5JbWFnZUVsZW1lbnQoKSkgeworICAgICAg ICAgICAgTW91c2VFdmVudCogbW91c2VFdmVudCA9IHN0YXRpY19jYXN0PE1vdXNlRXZlbnQqPihl dmVudCk7CisgICAgICAgICAgICBpZiAobW91c2VFdmVudC0+YnV0dG9uKCkgPT0gTGVmdEJ1dHRv bikgeworICAgICAgICAgICAgICAgIHRvSFRNTFBsdWdJbkltYWdlRWxlbWVudCh0aGlzKS0+dXNl ckRpZENsaWNrU25hcHNob3QobW91c2VFdmVudCwgZmFsc2UpOworICAgICAgICAgICAgICAgIGV2 ZW50LT5zZXREZWZhdWx0SGFuZGxlZCgpOworICAgICAgICAgICAgICAgIHJldHVybjsKKyAgICAg ICAgICAgIH0KKyAgICAgICAgfQorCiAgICAgICAgIGlmIChkaXNwbGF5U3RhdGUoKSA8IFBsYXlp bmcpCiAgICAgICAgICAgICByZXR1cm47CiAgICAgfQo= 204658 2013-06-13 18:15:25 -0700 2013-06-13 18:21:28 -0700 Patch bug-117620-20130613181344.patch text/plain 3957 dino U3VidmVyc2lvbiBSZXZpc2lvbjogMTUxNTcyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNWY0MGQ3MTI5ODRjYjg5 OTBkMjlmMWE0Yjg5OTY2ZjcwODdkNzJkNi4uYmM1MGY3MzUzYWVkNDQxMjk2Zjk3OWVhYjg0YTc0 M2RmNmQwMzY1MCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIzIEBACisyMDEzLTA2LTEzICBEZWFu IEphY2tzb24gIDxkaW5vQGFwcGxlLmNvbT4KKworICAgICAgICBDbGlja2luZyBvbiBzbmFwc2hv dHRpbmcgcGx1Zy1pbnMgZG9lcyBub3QgcmVzdGFydCB0aGVtCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMTc2MjAKKyAgICAgICAgPHJkYXI6Ly9wcm9i bGVtLzEzODIxNzI5PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIEhUTUxQbHVnSW5FbGVtZW50IGhhcyBhbiBldmVudCBoYW5kbGVyIHdoaWNoIHdvdWxk IHJldHVybgorICAgICAgICBpbW1lZGlhdGVseSBpZiB0aGUgZWxlbWVudCB3YXMgbm90IHNuYXBz aG90dGVkLCByZXN0YXJ0aW5nCisgICAgICAgIG9yIHBsYXlpbmcuIFRoZXJlIGlzIGEgY2FzZSBp dCBtaXNzZWQsIHdoaWNoIHdhcyBzbmFwc2hvdHRpbmcKKyAgICAgICAgKFdhaXRpbmdGb3JTbmFw c2hvdCB0byBiZSBwcmVjaXNlKS4gSWYgd2UgZ2V0IGEgY2xpY2sgaW4gdGhpcworICAgICAgICBz dGF0ZSB0aGVuIGl0IHNob3VsZCBpbW1lZGlhdGVseSByZXN0YXJ0LgorCisgICAgICAgICogaHRt bC9IVE1MUGx1Z0luRWxlbWVudC5oOiBNb3ZlIGRlZmF1bHRFdmVudEhhbmRsZXIgdG8gcHVibGlj ICh3ZSBjYWxsIGl0IGZyb20gdGhlIGNoaWxkIGNsYXNzKS4KKyAgICAgICAgKiBodG1sL0hUTUxQ bHVnSW5JbWFnZUVsZW1lbnQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6SFRNTFBsdWdJbkltYWdl RWxlbWVudDo6ZGVmYXVsdEV2ZW50SGFuZGxlcik6IElmIHRoZXJlIGlzIGEgY2xpY2sgb24gYSBz bmFwc2hvdHRpbmcKKyAgICAgICAgcGx1Zy1pbiwgdGhlbiBjYWxsIHJlc3RhcnQuCisgICAgICAg ICogaHRtbC9IVE1MUGx1Z0luSW1hZ2VFbGVtZW50Lmg6IFZpcnR1YWwgZGVmYXVsdEV2ZW50SGFu ZGxlciBkZWNsYXJhdGlvbi4KKwogMjAxMy0wNi0xMyAgQW5kcmVhcyBLbGluZyAgPGFrbGluZ0Bh cHBsZS5jb20+CiAKICAgICAgICAgTW92ZSBOb2RlOjpzaG91bGRVc2VJbnB1dE1ldGhvZCgpIHRv IEVsZW1lbnQuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxQbHVnSW5FbGVt ZW50LmggYi9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxQbHVnSW5FbGVtZW50LmgKaW5kZXggMTdi Yjc0MGJiZjIyNGEyYTZlOGYwMTU5MmRkOThmMzc2ZGEzYmEwMy4uYmM4MmJkMzZiMTRhM2E3ODkz NzIyMjg1NzlmNGRkNzFjZTAwM2RhZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvaHRtbC9I VE1MUGx1Z0luRWxlbWVudC5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTFBsdWdJbkVs ZW1lbnQuaApAQCAtODAsNiArODAsOCBAQCBwdWJsaWM6CiAKICAgICB2aXJ0dWFsIGJvb2wgaXNQ bHVnSW5JbWFnZUVsZW1lbnQoKSBjb25zdCB7IHJldHVybiBmYWxzZTsgfQogCisgICAgdmlydHVh bCB2b2lkIGRlZmF1bHRFdmVudEhhbmRsZXIoRXZlbnQqKTsKKwogcHJvdGVjdGVkOgogICAgIEhU TUxQbHVnSW5FbGVtZW50KGNvbnN0IFF1YWxpZmllZE5hbWUmIHRhZ05hbWUsIERvY3VtZW50Kik7 CiAKQEAgLTk5LDggKzEwMSw2IEBAIHByaXZhdGU6CiAKICAgICB2aXJ0dWFsIGJvb2wgYXJlQXV0 aG9yU2hhZG93c0FsbG93ZWQoKSBjb25zdCBPVkVSUklERSB7IHJldHVybiBmYWxzZTsgfQogCi0g ICAgdmlydHVhbCB2b2lkIGRlZmF1bHRFdmVudEhhbmRsZXIoRXZlbnQqKTsKLQogICAgIHZpcnR1 YWwgUmVuZGVyV2lkZ2V0KiByZW5kZXJXaWRnZXRGb3JKU0JpbmRpbmdzKCkgY29uc3QgPSAwOwog CiAgICAgdmlydHVhbCBib29sIHN1cHBvcnRzRm9jdXMoKSBjb25zdCBPVkVSUklERTsKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTFBsdWdJbkltYWdlRWxlbWVudC5jcHAgYi9T b3VyY2UvV2ViQ29yZS9odG1sL0hUTUxQbHVnSW5JbWFnZUVsZW1lbnQuY3BwCmluZGV4IDMxYTNm MzRlYzZhYTcyMDcyMWZkMzAwMWVkYjA5ZjViNGRkYjBiMjEuLjNkNWQ4NGY5YmI5Mzc3ZmRkMTU5 YjNhYzlkZDE4NmVkZDhiOGEyYmMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRN TFBsdWdJbkltYWdlRWxlbWVudC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MUGx1 Z0luSW1hZ2VFbGVtZW50LmNwcApAQCAtMjMsNiArMjMsOCBAQAogCiAjaW5jbHVkZSAiQ2hyb21l LmgiCiAjaW5jbHVkZSAiQ2hyb21lQ2xpZW50LmgiCisjaW5jbHVkZSAiRXZlbnQuaCIKKyNpbmNs dWRlICJFdmVudEhhbmRsZXIuaCIKICNpbmNsdWRlICJGcmFtZS5oIgogI2luY2x1ZGUgIkZyYW1l TG9hZGVyLmgiCiAjaW5jbHVkZSAiRnJhbWVMb2FkZXJDbGllbnQuaCIKQEAgLTcwNyw0ICs3MDks MjAgQEAgdm9pZCBIVE1MUGx1Z0luSW1hZ2VFbGVtZW50OjpzdWJmcmFtZUxvYWRlckRpZENyZWF0 ZVBsdWdJbihjb25zdCBXaWRnZXQqIHdpZGdldCkKICAgICB9CiB9CiAKK3ZvaWQgSFRNTFBsdWdJ bkltYWdlRWxlbWVudDo6ZGVmYXVsdEV2ZW50SGFuZGxlcihFdmVudCogZXZlbnQpCit7CisgICAg UmVuZGVyT2JqZWN0KiByID0gcmVuZGVyZXIoKTsKKyAgICBpZiAociAmJiByLT5pc0VtYmVkZGVk T2JqZWN0KCkpIHsKKyAgICAgICAgaWYgKGRpc3BsYXlTdGF0ZSgpID09IFdhaXRpbmdGb3JTbmFw c2hvdCAmJiBldmVudC0+dHlwZSgpID09IGV2ZW50TmFtZXMoKS5jbGlja0V2ZW50ICYmIGlzUGx1 Z0luSW1hZ2VFbGVtZW50KCkpIHsKKyAgICAgICAgICAgIE1vdXNlRXZlbnQqIG1vdXNlRXZlbnQg PSBzdGF0aWNfY2FzdDxNb3VzZUV2ZW50Kj4oZXZlbnQpOworICAgICAgICAgICAgaWYgKG1vdXNl RXZlbnQtPmJ1dHRvbigpID09IExlZnRCdXR0b24pIHsKKyAgICAgICAgICAgICAgICB1c2VyRGlk Q2xpY2tTbmFwc2hvdChtb3VzZUV2ZW50LCB0cnVlKTsKKyAgICAgICAgICAgICAgICBldmVudC0+ c2V0RGVmYXVsdEhhbmRsZWQoKTsKKyAgICAgICAgICAgICAgICByZXR1cm47CisgICAgICAgICAg ICB9CisgICAgICAgIH0KKyAgICB9CisgICAgSFRNTFBsdWdJbkVsZW1lbnQ6OmRlZmF1bHRFdmVu dEhhbmRsZXIoZXZlbnQpOworfQorICAgIAogfSAvLyBuYW1lc3BhY2UgV2ViQ29yZQpkaWZmIC0t Z2l0IGEvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MUGx1Z0luSW1hZ2VFbGVtZW50LmggYi9Tb3Vy Y2UvV2ViQ29yZS9odG1sL0hUTUxQbHVnSW5JbWFnZUVsZW1lbnQuaAppbmRleCAxNWFjNTA0ZWQ5 ZjNjNDE2OGUzZWVhMTU1MzI4NTQ1ZWZhMTVlMWY3Li5kZDgxNTZlNGI2MGFmMDNlMjM5OWYzMzYy Yjc0YmEyMWZhNTJhNDc5IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxQbHVn SW5JbWFnZUVsZW1lbnQuaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxQbHVnSW5JbWFn ZUVsZW1lbnQuaApAQCAtMTI1LDYgKzEyNSw4IEBAIHByb3RlY3RlZDoKIAogICAgIHZpcnR1YWwg Ym9vbCBpc1Jlc3RhcnRlZFBsdWdpbigpIGNvbnN0IE9WRVJSSURFIHsgcmV0dXJuIG1faXNSZXN0 YXJ0ZWRQbHVnaW47IH0KIAorICAgIHZpcnR1YWwgdm9pZCBkZWZhdWx0RXZlbnRIYW5kbGVyKEV2 ZW50Kik7CisKIHByaXZhdGU6CiAgICAgdmlydHVhbCBSZW5kZXJPYmplY3QqIGNyZWF0ZVJlbmRl cmVyKFJlbmRlckFyZW5hKiwgUmVuZGVyU3R5bGUqKTsKICAgICB2aXJ0dWFsIGJvb2wgd2lsbFJl Y2FsY1N0eWxlKFN0eWxlQ2hhbmdlKTsK