[Webkit-unassigned] [Bug 90611] contenteditable justify commands applied to next paragraph as well

[bugzilla-daemon at webkit.org]

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


Ryosuke Niwa <rniwa at webkit.org> changed:

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




--- Comment #16 from Ryosuke Niwa <rniwa at webkit.org>  2013-09-30 12:44:14 PST ---
(From update of attachment 213010)
View in context: https://bugs.webkit.org/attachment.cgi?id=213010&action=review

> Source/WebCore/ChangeLog:18
> +        (WebCore::ApplyStyleCommand::applyBlockStyle):Ignore the justify
> +        command for paragraph if selection is not part of it.

Part of what?

>> Source/WebCore/editing/ApplyStyleCommand.cpp:274
>> +    VisiblePosition beyondEnd((visibleEnd.deepEquivalent().deprecatedEditingOffset() || visibleEnd == paragraphStart) ? endOfParagraph(visibleEnd).next() : visibleEnd);
> 
> Can you do this without using anything with “deprecated” in its name?

This change doesn't make much sense. What does the deprecated editing offset being non-zero to do with visibleEnd == paragraphStart?

> LayoutTests/editing/execCommand/contenteditable-justify-next-paragraph.html:4
> +<script type="text/javascript">

There is no need to specify type in HTML5.

> LayoutTests/editing/execCommand/contenteditable-justify-next-paragraph.html:12
> +    var elem1 = document.getElementById("p1");
> +    var elem2 = document.getElementById("p2");

Please don't use abbreviations like elem and p. Spell out words.

> LayoutTests/editing/execCommand/contenteditable-justify-next-paragraph.html:14
> +    var selection = window.getSelection();
> +    selection.setBaseAndExtent(elem1, 0, elem2, 0);

There is no need for the local variable here and "window.".

> LayoutTests/editing/execCommand/contenteditable-justify-next-paragraph.html:18
> +    document.getElementById("result").innerHTML=  "Final Result:<br>" +  "1st Paragraph:  " + elem1.getAttribute("style") +  "<br>" + str;

Why isn't this using dump-as-markup or js-test-pre.js?

> LayoutTests/editing/execCommand/contenteditable-justify-next-paragraph.html:30
> +<p id="result"> </p>

Why do we have a space here?

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