51164 2010-12-15 22:36:55 -0800 execCommand('styleWithCSS') is ignored when selection isn't inside a contenteditable area 2010-12-16 15:39:52 -0800 1 1 1 Unclassified WebKit HTML Editing 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 rniwa rniwa abarth darin enrica eric jparent justin.garcia kalman ojan rolandsteiner webkit.review.bot oldest_to_newest 323054 0 rniwa 2010-12-15 22:36:55 -0800 WebKit ignores document.execCommand('styleWithCSS', false, true/false) when the document's selection isn't inside a contenteditable area. Since styleWithCSS doesn't modify DOM, WebKit should respect calls even when the selection isn't in an editable region. 323055 1 76735 rniwa 2010-12-15 22:42:42 -0800 Created attachment 76735 demo 323185 2 darin 2010-12-16 08:24:16 -0800 (In reply to comment #0) > styleWithCSS doesn't modify DOM I didn’t know that! I thought that it added style attributes to DOM elements. 323235 3 rniwa 2010-12-16 10:01:26 -0800 (In reply to comment #2) > (In reply to comment #0) > > styleWithCSS doesn't modify DOM > > I didn’t know that! I thought that it added style attributes to DOM elements. Subsequent execCommands DO modify DOM by inserting spans and adding style attributes but styleWithCSS itself doesn't. It's simply a flag in Editor class: http://trac.webkit.org/browser/trunk/WebCore/editing/EditorCommand.cpp?rev=73886#L982 http://trac.webkit.org/browser/trunk/WebCore/editing/Editor.h?rev=73886#L168 And it's not unusual for someone to call StyleWithCSS without putting caret inside an editable region. For example, text editor can call it right up front to enforce the use of CSS. We just had that bug in browser scope test (it was calling styleWithCSS before setting up selection). And apparently, we're the only browser that requires caret/selection being inside an editable region for StyleWithCSS. 323245 4 darin 2010-12-16 10:14:04 -0800 I see. This command simply sets a flag. This should be very easy to fix! 323269 5 jparent 2010-12-16 10:45:30 -0800 There is a little bit of subtlety here ... what happens if a document has 5 separate editable roots, the selection is in none of them, and you call execCommand('styleWithCSS')? Does it effect all of them? 323273 6 jparent 2010-12-16 10:47:32 -0800 Anyway, making this always enabled looks as easy as changing enabledInRichlyEditableText to enabled in { "StyleWithCSS", { executeStyleWithCSS, supported, enabledInRichlyEditableText, stateStyleWithCSS, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } }, in WebCore/editing/EditorCommand.cpp 323274 7 darin 2010-12-16 10:47:45 -0800 (In reply to comment #5) > There is a little bit of subtlety here ... what happens if a document has 5 separate editable roots, the selection is in none of them, and you call execCommand('styleWithCSS')? Does it effect all of them? We don’t have any per-editable-root settings. I presume this sets a per-document flag in WebKit now and should continue to do so. I’d imagine this is how ti works in other browsers as well. 323289 8 rniwa 2010-12-16 11:02:12 -0800 (In reply to comment #6) > Anyway, making this always enabled looks as easy as changing enabledInRichlyEditableText to enabled in > > { "StyleWithCSS", { executeStyleWithCSS, supported, enabledInRichlyEditableText, stateStyleWithCSS, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } }, Indeed. I'm going to post a patch in a minute. (In reply to comment #7) > (In reply to comment #5) > > There is a little bit of subtlety here ... what happens if a document has 5 separate editable roots, the selection is in none of them, and you call execCommand('styleWithCSS')? Does it effect all of them? > > We don’t have any per-editable-root settings. I presume this sets a per-document flag in WebKit now and should continue to do so. I’d imagine this is how ti works in other browsers as well. What Darin said. I think a lot of developers don't realize this because most of editing apps use an iframe + design mode to work-around Firefox's quirk to modify the editing host. 323304 9 76792 rniwa 2010-12-16 11:24:05 -0800 Created attachment 76792 fixes the bug 323326 10 rniwa 2010-12-16 11:59:11 -0800 Thanks for the review, Darin. Landing now. 323332 11 76792 rniwa 2010-12-16 12:02:09 -0800 Comment on attachment 76792 fixes the bug Clearing flags on attachment: 76792 Committed r74204: <http://trac.webkit.org/changeset/74204> 323333 12 rniwa 2010-12-16 12:02:14 -0800 All reviewed patches have been landed. Closing bug. 323480 13 webkit.review.bot 2010-12-16 15:39:52 -0800 http://trac.webkit.org/changeset/74204 might have broken Leopard Intel Debug (Tests) 76735 2010-12-15 22:42:42 -0800 2010-12-15 22:42:42 -0800 demo stylewithcss.html text/html 698 rniwa PCFET0NUWVBFIGh0bWw+CjxodG1sPgo8Ym9keT4KPHA+VGhpcyBpcyBhIHRlc3Qgb2Ygc3R5bGVX aXRoQ1NTLjwvcD4KPGRpdiBpZD0idGVzdCIgY29udGVudGVkaXRhYmxlPgpoZWxsbwo8L2Rpdj4K PHNjcmlwdD4KCnZhciB0ZXN0ID0gZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQoJ3Rlc3QnKTsKZG9j dW1lbnQuZXhlY0NvbW1hbmQoJ3N0eWxlV2l0aENTUycsIGZhbHNlLCB0cnVlKTsKd2luZG93Lmdl dFNlbGVjdGlvbigpLnNlbGVjdEFsbENoaWxkcmVuKHRlc3QpOwpkb2N1bWVudC5leGVjQ29tbWFu ZCgnYm9sZCcsIGZhbHNlLCBudWxsKTsKdmFyIGZpcnN0UmVzdWx0ID0gdGVzdC5pbm5lckhUTUw7 CmRvY3VtZW50LmV4ZWNDb21tYW5kKCd1bmRvJywgZmFsc2UsIG51bGwpOwpkb2N1bWVudC5leGVj Q29tbWFuZCgnc3R5bGVXaXRoQ1NTJywgZmFsc2UsIHRydWUpOwpkb2N1bWVudC5leGVjQ29tbWFu ZCgnYm9sZCcsIGZhbHNlLCBudWxsKTsKdmFyIHJlc3VsdCA9ICdQQVNTJzsKaWYgKGZpcnN0UmVz dWx0ICE9IHRlc3QuaW5uZXJIVE1MKQogICAgcmVzdWx0ID0gJ0ZBSUw6ICcgKyBmaXJzdFJlc3Vs dCArICcgZG9lcyBub3QgbWF0Y2ggJyArIHRlc3QuaW5uZXJIVE1MOwpkb2N1bWVudC5ib2R5LmFw cGVuZENoaWxkKGRvY3VtZW50LmNyZWF0ZVRleHROb2RlKHJlc3VsdCkpOwoKPC9zY3JpcHQ+Cjwv Ym9keT4KPC9odG1sPgo= 76792 2010-12-16 11:24:05 -0800 2010-12-16 12:02:09 -0800 fixes the bug bug-51164-20101216112404.patch text/plain 5007 rniwa ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCA4NWJhYzdlYmUxMGZmMzJhODZkY2U0OTgzYjZmZDIxNjkyNWIzNzNhLi43M2FiYThl NDMwOWJmM2EwZTdkMWRlZGNkMDMwZDQxZDNkNDAxNDRmIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0 cy9DaGFuZ2VMb2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAK KzIwMTAtMTItMTYgIFJ5b3N1a2UgTml3YSAgPHJuaXdhQHdlYmtpdC5vcmc+CisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgZXhlY0NvbW1hbmQoJ3N0eWxl V2l0aENTUycpIGlzIGlnbm9yZWQgd2hlbiBzZWxlY3Rpb24gaXNuJ3QgaW5zaWRlIGEgY29udGVu dGVkaXRhYmxlIGFyZWEKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTUxMTY0CisKKyAgICAgICAgQWRkZWQgYSB0ZXN0IHRvIGVuc3VyZSBXZWJLaXQgYWNj ZXB0cyBTdHlsZVdpdGhDU1MgY29tbWFuZCBldmVuIGlmIHRoZSBjdXJyZW50IHNlbGVjdGlvbgor ICAgICAgICBpc24ndCBpbnNpZGUgYSBjb250ZW50ZWRpdGFibGUgcmVnaW9uLgorCisgICAgICAg ICogZWRpdGluZy9zdHlsZS9zdHlsZXdpdGhjc3Mtd2l0aG91dC1zZWxlY3Rpb24tZXhwZWN0ZWQu dHh0OiBBZGRlZC4KKyAgICAgICAgKiBlZGl0aW5nL3N0eWxlL3N0eWxld2l0aGNzcy13aXRob3V0 LXNlbGVjdGlvbi5odG1sOiBBZGRlZC4KKwogMjAxMC0xMi0xNSAgSmlhbiBMaSAgPGppYW5saUBj aHJvbWl1bS5vcmc+CiAKICAgICAgICAgVW5yZXZpZXdlZC4gTWFyayBhIGZhaWxlZCB0ZXN0IGlu IGNocm9taXVtIHRlc3QgZXhwZWN0YXRpb25zLgpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvZWRp dGluZy9zdHlsZS9zdHlsZXdpdGhjc3Mtd2l0aG91dC1zZWxlY3Rpb24tZXhwZWN0ZWQudHh0IGIv TGF5b3V0VGVzdHMvZWRpdGluZy9zdHlsZS9zdHlsZXdpdGhjc3Mtd2l0aG91dC1zZWxlY3Rpb24t ZXhwZWN0ZWQudHh0Cm5ldyBmaWxlIG1vZGUgMTAwNjQ0CmluZGV4IDAwMDAwMDAwMDAwMDAwMDAw MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAuLjQyZTMxNmIzOWUxZDI0MWZjODQ2ZDRjYWQ5ZmM1ODYy ZWVhYTU0MTYKLS0tIC9kZXYvbnVsbAorKysgYi9MYXlvdXRUZXN0cy9lZGl0aW5nL3N0eWxlL3N0 eWxld2l0aGNzcy13aXRob3V0LXNlbGVjdGlvbi1leHBlY3RlZC50eHQKQEAgLTAsMCArMSw0IEBA CitUaGlzIGlzIGEgdGVzdCBvZiBzdHlsZVdpdGhDU1MuCisKK2hlbGxvCitQQVNTCmRpZmYgLS1n aXQgYS9MYXlvdXRUZXN0cy9lZGl0aW5nL3N0eWxlL3N0eWxld2l0aGNzcy13aXRob3V0LXNlbGVj dGlvbi5odG1sIGIvTGF5b3V0VGVzdHMvZWRpdGluZy9zdHlsZS9zdHlsZXdpdGhjc3Mtd2l0aG91 dC1zZWxlY3Rpb24uaHRtbApuZXcgZmlsZSBtb2RlIDEwMDY0NAppbmRleCAwMDAwMDAwMDAwMDAw MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwLi4yN2FmN2ZlMWFhMDkwOTYzYjEwYTE0M2RlZDI0 NjMyYTcwZjM5NzA0Ci0tLSAvZGV2L251bGwKKysrIGIvTGF5b3V0VGVzdHMvZWRpdGluZy9zdHls ZS9zdHlsZXdpdGhjc3Mtd2l0aG91dC1zZWxlY3Rpb24uaHRtbApAQCAtMCwwICsxLDMwIEBACis8 IURPQ1RZUEUgaHRtbD4KKzxodG1sPgorPGJvZHk+Cis8cD5UaGlzIGlzIGEgdGVzdCBvZiBzdHls ZVdpdGhDU1MuPC9wPgorPGRpdiBpZD0idGVzdCIgY29udGVudGVkaXRhYmxlPgoraGVsbG8KKzwv ZGl2PgorPHNjcmlwdD4KKworaWYgKHdpbmRvdy5sYXlvdXRUZXN0Q29udHJvbGxlcikKKyAgICBs YXlvdXRUZXN0Q29udHJvbGxlci5kdW1wQXNUZXh0KCk7CisKK3ZhciB0ZXN0ID0gZG9jdW1lbnQu Z2V0RWxlbWVudEJ5SWQoJ3Rlc3QnKTsKK2RvY3VtZW50LmV4ZWNDb21tYW5kKCdzdHlsZVdpdGhD U1MnLCBmYWxzZSwgdHJ1ZSk7Cit3aW5kb3cuZ2V0U2VsZWN0aW9uKCkuc2VsZWN0QWxsQ2hpbGRy ZW4odGVzdCk7Citkb2N1bWVudC5leGVjQ29tbWFuZCgnYm9sZCcsIGZhbHNlLCBudWxsKTsKK3Zh ciBmaXJzdElubmVySFRNTCA9IHRlc3QuaW5uZXJIVE1MOworCitkb2N1bWVudC5leGVjQ29tbWFu ZCgndW5kbycsIGZhbHNlLCBudWxsKTsKK2RvY3VtZW50LmV4ZWNDb21tYW5kKCdzdHlsZVdpdGhD U1MnLCBmYWxzZSwgdHJ1ZSk7Citkb2N1bWVudC5leGVjQ29tbWFuZCgnYm9sZCcsIGZhbHNlLCBu dWxsKTsKKwordmFyIHJlc3VsdCA9ICdQQVNTJzsKK2lmIChmaXJzdElubmVySFRNTCAhPSB0ZXN0 LmlubmVySFRNTCkKKyAgICByZXN1bHQgPSAnRkFJTDogJyArIGZpcnN0UmVzdWx0ICsgJyBkb2Vz IG5vdCBtYXRjaCAnICsgdGVzdC5pbm5lckhUTUw7Citkb2N1bWVudC5ib2R5LmFwcGVuZENoaWxk KGRvY3VtZW50LmNyZWF0ZVRleHROb2RlKHJlc3VsdCkpOworCis8L3NjcmlwdD4KKzwvYm9keT4K KzwvaHRtbD4KZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VM b2cKaW5kZXggZTE2YzJjNTY5NzQ0MDNlNmM4Yjc4ZTc0NTk1YjY1YjhmZTA5ZDFhYi4uNGQyZDEz ODJkMDkzNzE4MzExMDhjNDdkMmFkYTIwYThiY2NlMmJlOCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9D aGFuZ2VMb2cKKysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNyBAQAorMjAxMC0x Mi0xNiAgUnlvc3VrZSBOaXdhICA8cm5pd2FAd2Via2l0Lm9yZz4KKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBleGVjQ29tbWFuZCgnc3R5bGVXaXRoQ1NT JykgaXMgaWdub3JlZCB3aGVuIHNlbGVjdGlvbiBpc24ndCBpbnNpZGUgYSBjb250ZW50ZWRpdGFi bGUgYXJlYQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 NTExNjQKKworICAgICAgICBGaXhlZCB0aGUgYnVnIGJ5IGFsd2F5cyBlbmFibGluZyB0aGUgU3R5 bGVXaXRoQ1NTIGNvbW1hbmQuCisKKyAgICAgICAgVGVzdDogZWRpdGluZy9zdHlsZS9zdHlsZXdp dGhjc3Mtd2l0aG91dC1zZWxlY3Rpb24uaHRtbAorCisgICAgICAgICogZWRpdGluZy9FZGl0b3JD b21tYW5kLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OmNyZWF0ZUNvbW1hbmRNYXApOiBTdHlsZVdp dGhDU1MgaXMgZW5hYmxlZCByYXRoZXIgdGhhbiBlbmFibGVkSW5SaWNobHlFZGl0YWJsZVRleHQu CisKIDIwMTAtMTItMTUgIE1hcnRpbiBSb2JpbnNvbiAgPG1yb2JpbnNvbkBpZ2FsaWEuY29tPgog CiAgICAgICAgIFJldmlld2VkIGJ5IEd1c3Rhdm8gTm9yb25oYSBTaWx2YS4KZGlmZiAtLWdpdCBh L1dlYkNvcmUvZWRpdGluZy9FZGl0b3JDb21tYW5kLmNwcCBiL1dlYkNvcmUvZWRpdGluZy9FZGl0 b3JDb21tYW5kLmNwcAppbmRleCBmYjVhOWQ2YThmMGExZTI2NTI1MDk5NmM1M2Q2MWQ1OTMyNWZj OGQzLi43YjdlYzdiZWYwYjVjZDhjMGQ5ZDNjYWFhNjcyZjBiNGJjODZlYmRmIDEwMDY0NAotLS0g YS9XZWJDb3JlL2VkaXRpbmcvRWRpdG9yQ29tbWFuZC5jcHAKKysrIGIvV2ViQ29yZS9lZGl0aW5n L0VkaXRvckNvbW1hbmQuY3BwCkBAIC0xNTAyLDcgKzE1MDIsNyBAQCBzdGF0aWMgY29uc3QgQ29t bWFuZE1hcCYgY3JlYXRlQ29tbWFuZE1hcCgpCiAgICAgICAgIHsgIlNlbGVjdFdvcmQiLCB7IGV4 ZWN1dGVTZWxlY3RXb3JkLCBzdXBwb3J0ZWRGcm9tTWVudU9yS2V5QmluZGluZywgZW5hYmxlZFZp c2libGVTZWxlY3Rpb24sIHN0YXRlTm9uZSwgdmFsdWVOdWxsLCBub3RUZXh0SW5zZXJ0aW9uLCBk b05vdEFsbG93RXhlY3V0aW9uV2hlbkRpc2FibGVkIH0gfSwKICAgICAgICAgeyAiU2V0TWFyayIs IHsgZXhlY3V0ZVNldE1hcmssIHN1cHBvcnRlZEZyb21NZW51T3JLZXlCaW5kaW5nLCBlbmFibGVk VmlzaWJsZVNlbGVjdGlvbiwgc3RhdGVOb25lLCB2YWx1ZU51bGwsIG5vdFRleHRJbnNlcnRpb24s IGRvTm90QWxsb3dFeGVjdXRpb25XaGVuRGlzYWJsZWQgfSB9LAogICAgICAgICB7ICJTdHJpa2V0 aHJvdWdoIiwgeyBleGVjdXRlU3RyaWtldGhyb3VnaCwgc3VwcG9ydGVkLCBlbmFibGVkSW5SaWNo bHlFZGl0YWJsZVRleHQsIHN0YXRlU3RyaWtldGhyb3VnaCwgdmFsdWVOdWxsLCBub3RUZXh0SW5z ZXJ0aW9uLCBkb05vdEFsbG93RXhlY3V0aW9uV2hlbkRpc2FibGVkIH0gfSwKLSAgICAgICAgeyAi U3R5bGVXaXRoQ1NTIiwgeyBleGVjdXRlU3R5bGVXaXRoQ1NTLCBzdXBwb3J0ZWQsIGVuYWJsZWRJ blJpY2hseUVkaXRhYmxlVGV4dCwgc3RhdGVTdHlsZVdpdGhDU1MsIHZhbHVlTnVsbCwgbm90VGV4 dEluc2VydGlvbiwgZG9Ob3RBbGxvd0V4ZWN1dGlvbldoZW5EaXNhYmxlZCB9IH0sCisgICAgICAg IHsgIlN0eWxlV2l0aENTUyIsIHsgZXhlY3V0ZVN0eWxlV2l0aENTUywgc3VwcG9ydGVkLCBlbmFi bGVkLCBzdGF0ZVN0eWxlV2l0aENTUywgdmFsdWVOdWxsLCBub3RUZXh0SW5zZXJ0aW9uLCBkb05v dEFsbG93RXhlY3V0aW9uV2hlbkRpc2FibGVkIH0gfSwKICAgICAgICAgeyAiU3Vic2NyaXB0Iiwg eyBleGVjdXRlU3Vic2NyaXB0LCBzdXBwb3J0ZWQsIGVuYWJsZWRJblJpY2hseUVkaXRhYmxlVGV4 dCwgc3RhdGVTdWJzY3JpcHQsIHZhbHVlTnVsbCwgbm90VGV4dEluc2VydGlvbiwgZG9Ob3RBbGxv d0V4ZWN1dGlvbldoZW5EaXNhYmxlZCB9IH0sCiAgICAgICAgIHsgIlN1cGVyc2NyaXB0IiwgeyBl eGVjdXRlU3VwZXJzY3JpcHQsIHN1cHBvcnRlZCwgZW5hYmxlZEluUmljaGx5RWRpdGFibGVUZXh0 LCBzdGF0ZVN1cGVyc2NyaXB0LCB2YWx1ZU51bGwsIG5vdFRleHRJbnNlcnRpb24sIGRvTm90QWxs b3dFeGVjdXRpb25XaGVuRGlzYWJsZWQgfSB9LAogICAgICAgICB7ICJTd2FwV2l0aE1hcmsiLCB7 IGV4ZWN1dGVTd2FwV2l0aE1hcmssIHN1cHBvcnRlZEZyb21NZW51T3JLZXlCaW5kaW5nLCBlbmFi bGVkVmlzaWJsZVNlbGVjdGlvbkFuZE1hcmssIHN0YXRlTm9uZSwgdmFsdWVOdWxsLCBub3RUZXh0 SW5zZXJ0aW9uLCBkb05vdEFsbG93RXhlY3V0aW9uV2hlbkRpc2FibGVkIH0gfSwK