Bug 112554 – Tests added in r141354 erroneously assume that spelling markers are not erased after a selection change

• Component: Tools / Tests
• Status: ASSIGNED

[Grzegorz Czajkowski]

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.

[Ryosuke Niwa]

Why don't we set shouldEraseMarkersAfterChangeSelection to false in the test then?

[Rouslan Solomakhin]

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.

[Rouslan Solomakhin]

(In reply to comment #1)
> Why don't we set shouldEraseMarkersAfterChangeSelection to false in the test then?

This works, too.

[Ryosuke Niwa]

Also see 113742.

[Artur Moryc]

>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

[Grzegorz Czajkowski]

(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.

[Artur Moryc]

Created attachment 200094 [details]
Proposed patch for disappearing spelling marker

[Artur Moryc]

Comment on attachment 200094 [details]
Proposed patch for disappearing spelling marker 

This patch should go to 115165