[Webkit-unassigned] [Bug 107171] shouldMoveCaretToHorizontalBoundaryWhenPastTopOrBottom should return false on Android

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=107171





--- Comment #19 from Chris Hopman <cjhopman at chromium.org>  2013-01-18 15:41:02 PST ---
(From update of attachment 183554)
View in context: https://bugs.webkit.org/attachment.cgi?id=183554&action=review

>> LayoutTests/ChangeLog:12
>> +        * editing/selection/test-if-should-move-caret-to-horizontal-boundary-forced-false.html: Added.
> 
> Please don't add a separate js file for the script tests. That's the old way. The new way is to put the script in the single html file.
> Also, this test will have a different result on Windows as well as Chromium Windows so you'll need to add test expectations there as well.

Sure, I'll combine them. I don't think this test has different results on Windows - see below.

> LayoutTests/editing/selection/script-tests/test-if-should-move-caret-to-horizontal-boundary-forced-false.js:43
> +    internals.settings.setEditingBehavior("mac");

My understanding is that this call will make the test use the mac editing behavior.
I.e. the test behavior will be the same for all platforms except for chromium-android (which forces at compile time that EditingBehavior::shouldMoveCaret[...] returns false regardless of what editing behavior is set).

>> LayoutTests/platform/chromium-android/TestExpectations:114
>> +fast/writing-mode/flipped-blocks-hit-test-line-edges.html [ WontFix ]
> 
> It doesn't seem like a good idea to regress these tests. If anything the said chromium issue should be fixed first.

These tests use internals.settings.setEditingBehavior("somePlatform") and then depend on EditingBehavior::shouldMoveCaret[...] to behave according to that platform's editing behavior. This is not the case if it is #ifdeffed for chromium-android to always return false. So I see 3 options:

1. Rebaseline the tests for the new behavior. I didn't like this because these tests use testPassed and testFailed and the new behavior has a lot of FAILs (i.e. for all the parts of the test that assume mac or linux editing behavior). That makes the expected behavior look wrong and I don't know of a good way to notify someone looking at that new expected behavior that the FAILs are actually "PASSes".

2. Change the test to detect that it's actually on chromium-android and then understand that they won't get mac or linux behavior for shouldMoveCaret[...] and change all their checks accordingly. With this approach, chromium-android would still need new baselines for these tests, but the expected behavior wouldn't look like it was wrong. However, I don't really like the idea of such an invasive change to the tests just to support this odd chromium-android behavior.

3. Let the tests fail and mark them WontFix. This is clean and we can have a notification at the same point saying what's wrong with the tests and when they should be fixed. However, it's major failing is that chromium-android loses the coverage of these tests.

I did (1) and was working on (2) when I decided to go with (3). You know better than me, though, so if you'd prefer one of the other 2 options (or something else I didn't list) that'll be fine with me.

The mentioned chromium issue is a rather long term goal.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.