Summary: | More is<> and downcast<>, less static_cast<> | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Darin Adler <darin> | ||||||||||||||
Component: | WebCore Misc. | Assignee: | Darin Adler <darin> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | buildbot, cdumez, sam, webkit-bug-importer | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
Darin Adler
2017-11-12 16:18:00 PST
Created attachment 326729 [details]
Patch
Seems to break GTK build. Created attachment 326733 [details]
Patch
New one should compile in GTK. Ready for review, I think. Created attachment 326738 [details]
Patch
GTK bubble still red. Comment on attachment 326738 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=326738&action=review r=me with comment. Please make sure GTK builds before landing. > Source/WebCore/html/HTMLFormElement.cpp:671 > + if (!event || !is<Element>(event->target())) Doesn’t this change behavior? If the target is a Text node that is a descendant of an HTMLFormControlElement, then we will return nullptr instead of the control ancestor. Or I am missing something? > Source/WebCore/html/HTMLFormElement.cpp:673 > + return lineageOfType<HTMLFormControlElement>(downcast<Element>(*event->target())).first(); I discussed this Antti and he agreed with should have a lineageOfType overload that takes in a Node. Comment on attachment 326738 [details] Patch Attachment 326738 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/5205827 New failing tests: fast/canvas/webgl/webgl2-runtime-flag.html fast/canvas/webgl/getBufferSubData-webgl1.html Created attachment 326739 [details]
Archive of layout-test-results from ews113 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 326738 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=326738&action=review >> Source/WebCore/html/HTMLFormElement.cpp:671 >> + if (!event || !is<Element>(event->target())) > > Doesn’t this change behavior? If the target is a Text node that is a descendant of an HTMLFormControlElement, then we will return nullptr instead of the control ancestor. Or I am missing something? Mostly event targets can’t be text nodes. They target the element, not the text node. But I can’t verify this easily, and so I will rewrite to start with the node instead. Easy to do that with the parentElement function for now, while waiting for lineageOfType. OK, so four things to resolve before landing: 1) Crashing WebGL tests due to a mistake in HTMLCanvasElement::createContextWebGL. I fixed that. 2) Compiling on GTK failing because I used nil instead of nullptr. I fixed that. 3) Node vs. Element in findSubmitElement. Fixing that now. 4) Crash in svg/animations/svgtransform-animation-discrete.html test; seems unlikely to be due to my changes. Will test and see what I can learn. Created attachment 326741 [details]
Patch
Uploaded a patch that probably fixes 1-3, testing locally for 4 soon. Created attachment 326742 [details]
Patch
Committed r224740: <https://trac.webkit.org/changeset/224740> |