[Webkit-unassigned] [Bug 28306] double-clicking a word inside <b> beside newline select two words

[bugzilla-daemon at webkit.org]

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





--- Comment #21 from MORITA Hajime <morrita at gmail.com>  2009-12-12 19:31:25 PST ---
(In reply to comment #18)

Thank you for your review.
I revised the patch again.

> Tab in change log here.
Fixed.

> The implementation of this function is wrong. The fact that whitespace is
> collapsible is not enough. You need to take style into account. The right way
> to do that is to call RenderStyle::isCollapsibleWhitespace, which gives the
> correct answer. It is almost never correct to call the global
> isCollapsibleWhitespace function.
>
> Also, the two line for body needs to have braces around it.
Fixed. 

And I Added some test cases that fails with last patch.
Using RenderStyle::isCollapsibleWhitespace() resolves the problem. 
Thank you to point it out!

> It is also unnecessarily inefficient to put the text into a string object. I
> would instead do this:
>
>     const UChar* characters = renderer->text()->characters();
> 
> And then index directly into the characters array.
Fixed.

> Also, for local variables we try to use words rather than abbreviations. The
> "i" is OK, but "str" is not so great. I also think "length" is fine, since the
> length is the length of the text.
Fixed.

> To test the different whitespace modes, we need test cases that cover editable
> text with the CSS white-space values, normal, pre, pre-wrap, pre-line, and
> nowrap, covering both spaces and newline characters, and ideally tabs too,
> although they should basically work the same as spaces.
Added test cases for differenct white-space style values and some other edge
conditions.

> I'm not entirely sure this change is OK. It's kind of strange to hardcode this
> behavior down at this low level. (snip)
Agreed, and revised the patch trying to clarify the intention.

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