Description
Daniel Bates
2017-11-28 09:34:54 PST
Created attachment 328039 [details]
Patch and layout test
Comment on attachment 328039 [details] Patch and layout test Attachment 328039 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/5427658 New failing tests: fast/forms/alternative-presentation-button/replacement.html Created attachment 328052 [details]
Archive of layout-test-results from ews101 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 328039 [details] Patch and layout test Attachment 328039 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5427759 New failing tests: fast/forms/alternative-presentation-button/replacement.html Created attachment 328053 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 328039 [details] Patch and layout test Attachment 328039 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/5427629 New failing tests: svg/custom/object-sizing-explicit-height.xhtml fast/forms/alternative-presentation-button/replacement.html Created attachment 328054 [details]
Archive of layout-test-results from ews114 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 328039 [details] Patch and layout test Attachment 328039 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5427716 New failing tests: fast/forms/alternative-presentation-button/replacement.html Created attachment 328055 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Created attachment 328136 [details]
Patch and layout test
We likely can simplify the logic for picking the element for alternative presentation in Editor::substituteWithAlternativePresentationButton(), including removing the need to expose Element::canAttachAuthorShadowRoot(). We are still iterating on the algorithm and I want to avoid a programmer error of inadvertently whitelisting an element that can have an author shadow root. Once we finalize the algorithm we will be in a position to determine whether it is necessary to expose Element::canAttachAuthorShadowRoot() to catch such errors. I added more test cases to LayoutTests/fast/forms/alternative-presentation-button/replacement.html and expect the EWS to fail this test. Will grab results from EWS and update patch.
Comment on attachment 328136 [details] Patch and layout test Attachment 328136 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/5459193 New failing tests: fast/forms/alternative-presentation-button/replacement.html Created attachment 328172 [details]
Archive of layout-test-results from ews102 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 328136 [details] Patch and layout test Attachment 328136 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5459194 New failing tests: fast/forms/alternative-presentation-button/replacement.html Created attachment 328174 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 328136 [details] Patch and layout test Attachment 328136 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/5459263 New failing tests: fast/forms/alternative-presentation-button/replacement.html Created attachment 328178 [details]
Archive of layout-test-results from ews115 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 328179 [details]
Patch and layout test
Comment on attachment 328179 [details] Patch and layout test Attachment 328179 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5460733 New failing tests: fast/forms/alternative-presentation-button/replacement.html Created attachment 328190 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 328179 [details] Patch and layout test Attachment 328179 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/5460623 New failing tests: fast/forms/alternative-presentation-button/replacement.html Created attachment 328192 [details]
Archive of layout-test-results from ews100 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 328179 [details] Patch and layout test Attachment 328179 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5460714 New failing tests: fast/forms/alternative-presentation-button/replacement.html Created attachment 328195 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 328179 [details] Patch and layout test Attachment 328179 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/5460707 New failing tests: svg/custom/object-sizing-explicit-height.xhtml fast/forms/alternative-presentation-button/replacement.html Created attachment 328196 [details]
Archive of layout-test-results from ews116 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 328350 [details]
Patch and layout test
Comment on attachment 328350 [details] Patch and layout test Attachment 328350 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/5488740 New failing tests: fast/forms/alternative-presentation-button/replacement.html Created attachment 328358 [details]
Archive of layout-test-results from ews103 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 328350 [details] Patch and layout test Attachment 328350 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5488818 New failing tests: fast/forms/alternative-presentation-button/replacement.html Created attachment 328359 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 328350 [details] Patch and layout test Attachment 328350 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/5488720 New failing tests: fast/forms/alternative-presentation-button/replacement.html Created attachment 328361 [details]
Archive of layout-test-results from ews112 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 328350 [details] Patch and layout test View in context: https://bugs.webkit.org/attachment.cgi?id=328350&action=review > Source/WebCore/editing/Editor.cpp:3866 > + for (auto* tagName : tagNames) { > + if (element.hasTagName(*tagName)) > + return true; > + } > + return false; Instead of for looping, we should create a NeverDestroyed HashSet. > Source/WebCore/editing/Editor.cpp:3868 > + auto findElementForAlternativePresentation = [&] (const Vector<Ref<Element>>& ancestorElements) { What's the point of having this inner lambda? It would a lot cleaner to have this function as a separate static local function. > Source/WebCore/editing/Editor.cpp:3904 > + // Fix up the the list of elements to hide so that we do not hide the element for alternative presentation. Instead of having a comment like this, it's a lot better to extract a static local function of a descriptive name like elementsToHideExcludingAlternativePresentation. > LayoutTests/fast/forms/alternative-presentation-button/replacement.html:43 > + <a>a</a> You should add a test case with href: <a href=""> By the way, can't we use dump-as-markup.js in these tests? That would make the output a lot more readable & platform independent. Created attachment 328618 [details] Patch for landing Patch that incorporates the feedback from Ryosuke Niwa. This patch depends on the patch for bug #180381. Otherwise, the layout test fast/forms/alternative-presentation-button/replacement.html will crash due to an assertion failure when executing "document.body.removeChild(testContainer);" The approach for the alternative presentation button has since changed. This bug is no longer applicable. |