[Webkit-unassigned] [Bug 33769] [Chromium] layout_test on linux fails on doubleclick-beside-cr-span.html

[bugzilla-daemon at webkit.org]

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


Shinichiro Hamaji <hamaji at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #46758|review?                     |review-
               Flag|                            |




--- Comment #3 from Shinichiro Hamaji <hamaji at chromium.org>  2010-01-17 17:52:54 PST ---
(From update of attachment 46758)
Thanks for fixing! Some nitpicks and questions.

> diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
> +        [Chromium] layout_test on linux fails on doubleclick-beside-cr-span.html
> +        
> +        https://bugs.webkit.org/show_bug.cgi?id=33769

It would be better to explain how did you fix this failure.

> -    eventSender.mouseMoveTo(pos.x + 2, pos.y + 2);
> +    // We choose value "4" heuristically - will longer than single tab, shorter than a word.
> +    // So cases should follow this assumption.

I'd say "Out test cases will have at most a single leading tab followed by a
word whose length is more than 4. So, pos.x + CHAR_WIDTH * 4 always hits the
words" or something.

> +    eventSender.mouseMoveTo(pos.x + CHAR_WIDTH*4, pos.y + LINE_HEIGHT/2);

I think we usually put white-spaces around '*' and '/'.

> @@ -160,14 +155,14 @@ function runTests()
>          doTest("totest_multiple_word_in_span", "select9 ");
>          doTest("totest_word_before_here_in_line", "select10 ");
>          doTest("totest_span_first_half", "select11 ");
> -        doTest("totest_span_second_half", "select12 ");
> +        doTest("totest_span_second_half", "selectHere12 ");

I'm not sure why this "Here" is necessary. I think it would be nice if you put
a comment for this line.

> -<div style="width:100pt">
> -abcd efgh ijkl mnop qrst sel<b id="totest_span_second_half">ect12</b> notyet
> +<div style="width:150pt">
> +abcd efgh ijkl mnop qrst uvwx yz123 sel<b id="totest_span_second_half">ectHere12</b> notyet

Why did we change here?

> -abcd efgh ijkl mnop<b id="totest_multiple_whitespaces_in_pre">    select6    </b>nottoselect
> +abcd efgh ijkl mnop<b id="totest_multiple_whitespaces_in_pre">   select6   </b>nottoselect

Ditto.

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