| Summary: | Document's execCommand() / queryCommand*() should throw an exception on non-HTML/XHTML documents | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | zyscoder <zyscoder> | ||||||||||||||
| Component: | WebKit Misc. | Assignee: | Chris Dumez <cdumez> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | achristensen, berto, cdumez, cgarcia, darin, esprehn+autocc, ews-watchlist, ggaren, gustavo, kangil.han, sam, webkit-bug-importer, zimmermann | ||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||
| Version: | WebKit Local Build | ||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||
| OS: | Linux | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
Created attachment 431556 [details]
Patch
Created attachment 431561 [details]
Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API Created attachment 431575 [details]
Patch
Created attachment 431577 [details]
Patch
Comment on attachment 431577 [details]
Patch
Why are we keeping all these pointless calls to execCommand in SVG tests?
(In reply to Darin Adler from comment #6) > Comment on attachment 431577 [details] > Patch > > Why are we keeping all these pointless calls to execCommand in SVG tests? Fair point, I hesitated. I think I'll drop them then. Created attachment 431614 [details]
Patch
Created attachment 431615 [details]
Patch
Committed r278974 (238900@main): <https://commits.webkit.org/238900@main> This broke most SVG text selection tests in LayoutTests/svg/text -- but it was hard to notice, since the failures are only visible when running pixel tests. They need to be converted to ref tests in the future :-( Anyhow, the purpose of the tests e.g. text-align-01-b.svg is to select all text and verify that the selection rects are correct -- now no more text is selected. How to select all text in SVG documents w/o using execCommand? Here's a full list of affected tests: svg/text/foreignObject-text-clipping-bug.xml svg/text/text-align-01-b.svg svg/text/text-align-02-b.svg svg/text/text-align-03-b.svg svg/text/text-align-04-b.svg svg/text/text-align-05-b.svg svg/text/text-align-06-b.svg svg/text/text-deco-01-b.svg svg/text/text-fonts-01-t.svg svg/text/text-fonts-02-t.svg svg/text/text-intro-05-t.svg svg/text/text-path-01-b.svg svg/text/text-spacing-01-b.svg svg/text/text-text-01-b.svg svg/text/text-text-03-b.svg svg/text/text-text-04-t.svg svg/text/text-text-05-t.svg svg/text/text-text-07-t.svg svg/text/text-text-08-b.svg svg/text/text-tref-01-b.svg svg/text/text-tselect-01-b.svg svg/text/text-tspan-01-b.svg svg/text/text-ws-01-t.svg svg/text/text-ws-02-t.svg (In reply to Nikolas Zimmermann from comment #12) > This broke most SVG text selection tests in LayoutTests/svg/text -- but it > was hard to notice, since the failures are only visible when running pixel > tests. > > They need to be converted to ref tests in the future :-( > Anyhow, the purpose of the tests e.g. text-align-01-b.svg is to select all > text and verify that the selection rects are correct -- now no more text is > selected. How to select all text in SVG documents w/o using execCommand? I guess that's not something that's supported in other browsers (selecting all text in an SVG document). It looks like the tests in questions were dropped in Blink. Instead of dropping them, I intentionally dropped the selection logic from those tests. |
Steps to reproduce: (1) Open a tab and navigate to any URL; (2) Open Devtools and run the following code in the Console: ``` new Document().queryCommandSupported('str'); ``` Actual results: This code would be evaluated successfully and return a value of false, without throwing any exceptions. Expected results: Both in Chrome and Firefox throw expcetions: Chrome: `DOMException: Failed to execute 'queryCommandSupported' on 'Document': queryCommandSupported is only supported on HTML documents.` Firefox: `queryCommandSupported is only supported on HTML documents.` It seems better to align with Chrome and Firefox.