112554 2013-03-18 03:53:30 -0700 Tests added in r141354 erroneously assume that spelling markers are not erased after a selection change 2013-04-30 02:56:31 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified ASSIGNED P2 Normal --- 108528 1 g.czajkowski g.czajkowski a.moryc morrita rniwa rouslan+webkit tony oldest_to_newest 856965 0 g.czajkowski 2013-03-18 03:53:30 -0700 There's 'shouldEraseMarkersAfterChangeSelection' setting which is exposed to port implementation whether we should erase markers after selection change or not. By default this setting is set on true. Chromium and Mac (at least for wk2) prefer not erasing markers so they return false for this setting. A newly added tests (checked for spelling-double-clicked-word.html) assume that selection (as a result of double click on misspelled word) doesn't erase spelling markers. Here is the code: spellingMarkerRange = internals.markerRangeForNode(textNode, "spelling", 0); <--- OK, marker exists .... // Double-click the misspelled word eventSender.mouseDown(); eventSender.mouseUp(); eventSender.mouseDown(); eventSender.mouseUp(); shouldBeEqualToString("window.getSelection().toString()", "wellcome"); <--- Ok, verifies selection after double click spellingMarkerRange = internals.markerRangeForNode(textNode, "spelling", 0); <--- spellingMarkerRange is NULL for platforms whose erase markers after selection change. I'd like to ask you what's the best way of fixing this issue for other Platforms? It seems reasonably to check whether ports erase markers or not (we could just check spellingMarkerRange object). As a result we will have to deliver new baseline for other ports (at lest EFL). Of course we could implement this behaviour similarly to Chromium and Mac but I am not familiar with it. I couldn't see any visual changes while editing misspelled word in MiniBrowser. Thanks. 857133 1 rniwa 2013-03-18 08:20:36 -0700 Why don't we set shouldEraseMarkersAfterChangeSelection to false in the test then? 857196 2 rouslan+webkit 2013-03-18 09:29:33 -0700 I think that this tests is checking for more things that necessary. I think that you can safely remove the failing assumption from the test and the expectation file. 857197 3 rouslan+webkit 2013-03-18 09:30:09 -0700 (In reply to comment #1) > Why don't we set shouldEraseMarkersAfterChangeSelection to false in the test then? This works, too. 867521 4 rniwa 2013-04-01 21:02:55 -0700 Also see 113742. 880636 5 a.moryc 2013-04-24 08:37:40 -0700 >I couldn't see any visual changes while editing misspelled word in MiniBrowser. What I have noticed so far in MiniBrowser with respect to the returned value of shouldEraseMarkersAfterChangeSelection is: 1) true the marker for the misspelled word disappears while the mouse cursor is being navigated along the misspelled word 2) false the misspelled word remains underlined regardless the mouse cursor position 881104 6 g.czajkowski 2013-04-24 23:56:22 -0700 (In reply to comment #5) > >I couldn't see any visual changes while editing misspelled word in MiniBrowser. > > What I have noticed so far in MiniBrowser with respect to the returned value of shouldEraseMarkersAfterChangeSelection is: > > 1) true > > the marker for the misspelled word disappears while the mouse cursor is being navigated along the misspelled word > > > 2) false > > the misspelled word remains underlined regardless the mouse cursor position Thanks Artur for checking the setting. IMO it seems reasonable to implement 'shouldEraseMarkersAfterChangeSelection' for EFL similarly to Mac (no to erase spelling markers after selection/cursor change). (In reply to comment #2) > I think that this tests is checking for more things that necessary. I think that you can safely remove the failing assumption from the test and the expectation file. Agree. It will be helpful for ports that prefer default behaviour. 882993 7 200094 a.moryc 2013-04-30 02:46:48 -0700 Created attachment 200094 Proposed patch for disappearing spelling marker 882996 8 200094 a.moryc 2013-04-30 02:56:31 -0700 Comment on attachment 200094 Proposed patch for disappearing spelling marker This patch should go to 115165 200094 2013-04-30 02:46:48 -0700 2013-04-30 02:56:30 -0700 Proposed patch for disappearing spelling marker SpellingMarkerDisappearingCorrection.patch text/plain 1746 a.moryc Y29tbWl0IGRmNTA3YTNlMTI1MGEyZGMxMGEwZGQzMDcwNDkwOTdjNTBhZDM2ZmUKQXV0aG9yOiBB cnR1ciBNb3J5YyA8YS5tb3J5Y0BzYW1zdW5nLmNvbT4KRGF0ZTogICBNb24gQXByIDI5IDE2OjE2 OjIzIDIwMTMgKzAyMDAKCiAgICBkaXNhcHBlYXJpbmcgbWFya2VyIGZvciBtaXNzcGVsbGVkIHdv cmRzIGNvcnJlY3Rpb24KICAgIAogICAgQ2hhbmdlLUlkOiBJODk4MjdmMGMzMWE1MGE0MjJhYmYw ZGNiMGQyNjE2YWNmMTgwNGI1MAoKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxv ZyBiL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwppbmRleCA4ZjNhMTgwLi4wOTU3ZDU2IDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTMtMDQtMjkgIEFydHVyIE1vcnljICA8YS5tb3J5 Y0BzYW1zdW5nLmNvbT4KKworICAgICAgICBbV0syXVtFRkxdIFNwZWxsaW5nIG1hcmtlciBkaXNh cHBlYXJzIHdoaWxlIHNlbGVjdGlvbiBpcyBiZWluZyBjaGFuZ2VkCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMTUxNjUKKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGUgbWFya2VyIGZvciBhIG1pc3NwZWxs ZWQgd29yZCBkaXNhcHBlYXJzIHdoaWxlIHRoZSBjdXJzb3IgaXMgYmVpbmcgbmF2aWdhdGVkIGFs b25nIHRoZSB3b3JkLgorCisgICAgICAgICogV2ViUHJvY2Vzcy9XZWJDb3JlU3VwcG9ydC9XZWJF ZGl0b3JDbGllbnQuY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJFZGl0b3JDbGllbnQ6OnNob3Vs ZEVyYXNlTWFya2Vyc0FmdGVyQ2hhbmdlU2VsZWN0aW9uKToKKwogMjAxMy0wNC0yOSAgSmlud29v IFNvbmcgIDxqaW53b283LnNvbmdAc2Ftc3VuZy5jb20+CiAKICAgICAgICAgW1dLMl0gUmVtb3Zl IGJ1aWxkIHdhcm5pbmdzIGJ5IC1XdW51c2VkLXBhcmFtZXRlcgpkaWZmIC0tZ2l0IGEvU291cmNl L1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJDb3JlU3VwcG9ydC9XZWJFZGl0b3JDbGllbnQuY3BwIGIv U291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJDb3JlU3VwcG9ydC9XZWJFZGl0b3JDbGllbnQu Y3BwCmluZGV4IDkxYjQ3MzEuLjlmNDRmMDkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1dl YlByb2Nlc3MvV2ViQ29yZVN1cHBvcnQvV2ViRWRpdG9yQ2xpZW50LmNwcAorKysgYi9Tb3VyY2Uv V2ViS2l0Mi9XZWJQcm9jZXNzL1dlYkNvcmVTdXBwb3J0L1dlYkVkaXRvckNsaWVudC5jcHAKQEAg LTQwMSw3ICs0MDEsNyBAQCB2b2lkIFdlYkVkaXRvckNsaWVudDo6dGV4dFdpbGxCZURlbGV0ZWRJ blRleHRGaWVsZChFbGVtZW50KiBlbGVtZW50KQogYm9vbCBXZWJFZGl0b3JDbGllbnQ6OnNob3Vs ZEVyYXNlTWFya2Vyc0FmdGVyQ2hhbmdlU2VsZWN0aW9uKFdlYkNvcmU6OlRleHRDaGVja2luZ1R5 cGUgdHlwZSkgY29uc3QKIHsKICAgICAvLyBUaGlzIHByZXZlbnRzIGVyYXNpbmcgc3BlbGxpbmcg bWFya2VycyBvbiBPUyBYIExpb24gb3IgbGF0ZXIgdG8gbWF0Y2ggQXBwS2l0IG9uIHRoZXNlIE1h YyBPUyBYIHZlcnNpb25zLgotI2lmIFBMQVRGT1JNKE1BQykKKyNpZiBQTEFURk9STShNQUMpIHx8 IFBMQVRGT1JNKEVGTCkKICAgICByZXR1cm4gdHlwZSAhPSBUZXh0Q2hlY2tpbmdUeXBlU3BlbGxp bmc7CiAjZWxzZQogICAgIFVOVVNFRF9QQVJBTSh0eXBlKTsK