[Webkit-unassigned] [Bug 112275] TextIterator emits LF for a br element inside an empty input element

[bugzilla-daemon at webkit.org]

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


Aurimas Liutikas <aurimas at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #193167|1                           |0
        is obsolete|                            |




--- Comment #12 from Aurimas Liutikas <aurimas at chromium.org>  2013-03-14 14:10:08 PST ---
(From update of attachment 193167)
View in context: https://bugs.webkit.org/attachment.cgi?id=193167&action=review

>> Source/WebCore/editing/TextIterator.cpp:753
>>      RenderObject* r = node->renderer();
> 
> While we're at it, please rename r to renderer, and remove this comment that repeats the self-evident fact.

Done

>> Source/WebCore/editing/TextIterator.cpp:755
>> +    // Checking if this node is a <br> element within a shadow tree of an input element.
> 
> This comment repeats the code. Please remove it.

Done

>> Source/WebCore/editing/TextIterator.cpp:756
>> +    bool isNodeInsideInputShadowTree = node->isInShadowTree() && node->shadowHost()->toInputElement();
> 
> The boolean shouldn't be a question. Rename it to nodeIsInsideInputElement.

Done

>> LayoutTests/editing/text-iterator/script-tests/basic-iteration.js:35
>> +var input = testDocument.body.children[0];
> 
> I would prefer doing testDocument.querySelector('input') instead.

Done

>> LayoutTests/editing/text-iterator/script-tests/basic-iteration.js:37
>> +shouldBe('range.selectNodeContents(shadow); internals.rangeAsText(range)', '"b"');
> 
> It would have been better if we called internals.oldestShadowRoot(input) inside shouldBe.

Done

>> LayoutTests/editing/text-iterator/script-tests/basic-iteration.js:41
>> +mydiv.appendChild(mybr);
> 
> Please don't use variables names like mydiv and mybr.
> Also, it would have been better if we added a helper function to create and append a br element and then called that inside shouldBe.

Done

>> LayoutTests/editing/text-iterator/script-tests/basic-iteration.js:46
>> +var shadow = maindiv.webkitCreateShadowRoot();
> 
> We should create some helper function to do this and call it inside shouldBe.

Done

>> LayoutTests/editing/text-iterator/script-tests/basic-iteration.js:49
>> +shadow.appendChild(document.createElement('br'));
> 
> This should be called inside shouldBe.

Done

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