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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jul 14 15:16:00 PDT 2011


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





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

> LayoutTests/editing/selection/programmatic-selection-on-mac-is-directionless.html:14
> +        The first sentence. The second sentence. Three sentences.<br>

We can be more aggressive and have 1-3 words in each line.

> LayoutTests/editing/selection/programmatic-selection-on-mac-is-directionless.html:79
> +    macIsBackwardWinAndUnixAreForward = {'mac': 'backward', 'win': 'forward', 'unix': 'forward'}[currentPlatform];

This variable name repeats what RHS says.  I want to know what this variable is for instead.

> LayoutTests/editing/selection/programmatic-selection-on-mac-is-directionless.html:81
> +    allPlatformsAreForward = 'forward';
> +    allPlatformsAreBackward = 'backward';

Ditto.  Also, I don't see any benefit in having these variables around.  It only bloats the code as far as I can tell.

> LayoutTests/editing/selection/programmatic-selection-on-mac-is-directionless.html:100
> +        expectedResult = {'mac': 'e second sentence. Three sentences.\nAnother sentence. A test', 
> +                          'win': 'econd sentence. Three sentences.\nAnother sentence. A ', 
> +                          'unix': 'econd sentence. Three sentences.\nAnother sentence. A '};

I've got a feeling that these results are platform dependent because it depends on the width of words.  You should really make lines MUCH shorter (probably just one or two words) and set the selection at the beginning of line instead of in the middle of it.

> LayoutTests/editing/selection/programmatic-selection-on-mac-is-directionless.html:120
> +        if(listOfTestNodes[i].nodeName != '#text')

Space after for and if.

> LayoutTests/editing/selection/programmatic-selection-on-mac-is-directionless.html:121
> +    for(i = 0; i < listOfTestNodes.length; i++)
> +        if(listOfTestNodes[i].nodeName != '#text')
> +            runTestsOn(platform, listOfTestNodes[i]);

You need { } around the if-statement; i.e.
for (i = 0; i < listOfTestNodes.length; i++) {
    if (listOfTestNodes[i].nodeName != '#text')
        runTestsOn(platform, listOfTestNodes[i]);
}

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



More information about the webkit-unassigned mailing list