If a plug-in is snapshotting and the user clicks on it, it should immediately restart. <rdar://problem/13821729>
Created attachment 204657 [details] Patch
Comment on attachment 204657 [details] Patch Ignore this. I tried to sneak it by but it is bad coding.
Comment on attachment 204657 [details] 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?
Created attachment 204658 [details] Patch
(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 :(
Comment on attachment 204658 [details] 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.
Committed r151576: <http://trac.webkit.org/changeset/151576>
Comment on attachment 204658 [details] 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.
Follow-up commit for Darin's comments: https://trac.webkit.org/r151599