[Webkit-unassigned] [Bug 60529] Programmatically set selection should not have direction on Mac

[bugzilla-daemon at webkit.org]

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





--- Comment #14 from Ryosuke Niwa <rniwa at webkit.org>  2011-07-13 21:12:22 PST ---
(From update of attachment 100760)
View in context: https://bugs.webkit.org/attachment.cgi?id=100760&action=review

I don't think we need to split the test into two files.  If you're doing that, then you should probably share the test code by adding some js file into resources.  If not, then you can branch into two code paths by checking node.setSelectionRange because that's defined iff the node was input or textarea.  Alternatively, you can just check node.localName == 'div' since you're only using div in the other cases.

> LayoutTests/editing/selection/programmatic-selection-on-mac-is-directionless-1.html:13
> +    <div id="regular-div">Superfluous line.<br>Superfluous line.<br>The first sentence. The second sentence. Three sentences.<br>Another sentence. A fifth sentence. A sixth sentence.<br>Sentence seven. Sentence eight. The last sentence.<br>Superfluous line.<br>Superfluous line.</div> 

Can we just have 3 lines instead?  All these extra lines and sentences are making this code less readable.

> LayoutTests/editing/selection/programmatic-selection-on-mac-is-directionless-1.html:50
> +function makeTest(direction, granularity) {

I'd call this makeSelectionModifier.

>> LayoutTests/editing/selection/programmatic-selection-on-mac-is-directionless-1.html:76
>> +    runTestsOn(platform, document.getElementById('editable-div'));
> 
> I started off with a node-travesal like so:
> 
> for(i = 0; i < 2; i++)
>     runTestsOn(platform, document.getElementById('test').children[i];
> 
> but it gave me a wacky syntax error the second time through, complaining about '1' as a property to undefined. I messed with it for 15 minutes and opted for explicitly showing the two tags instead. It's the same number of lines and similarly clear what's going on, but if you'd like me to look into the for loop solution, I'll try harder at it :P

It's childNodes, not children!  You should also skip text nodes generated by whitespace around tags.

> LayoutTests/editing/selection/programmatic-selection-on-mac-is-directionless-2.html:61
> +        expectedResult = {'mac': 'Superfluous line. Superfluous line. The first sentence. The second sentence. Three sentences. Another sentence. A fifth', 

If we're special-casing input element then why do we need so much text in the first place?  It seems like we can trim it to be 1-2 words.

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