[Webkit-unassigned] [Bug 26991] [Gtk] get_n_selections and get_selection fail when selecting text across object boundaries

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

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




--- Comment #10 from Martin Robinson <mrobinson at webkit.org>  2010-09-07 14:52:35 PST ---
(From update of attachment 66734)
View in context: https://bugs.webkit.org/attachment.cgi?id=66734&action=prettypatch

Looks good, but I have a few concerns.

> WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1336
> +        return false;
Any reason you aren't using selection.firstRange() or selection.toNormalizedRange() here?

> WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1341
> +    for (TextIterator it(range.get()); !it.atEnd(); it.advance()) {
Would it be possible to replace this loop with Range::intersectsNode?

> WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1360
> +    RefPtr<Range> range = makeRange(selection.visibleStart(), selection.visibleEnd());
Again it seems like this should be selection.firstRange() or selection.toNormalizedRange().

> WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1376
> +            break;
Tricky loop logic. :/ I admit I'm not sure exactly what the range return value is supposed to represent, but could you take a non-loop approach, such as the one used in AccessibilityRenderObject::ariaSelectedTextDOMRange in WebCore/accessibility/AccessibilityRenderObject.cpp?

> WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1397
> +    if (selection_num)
There are lots of non-WebKit style variable names in this file, but we should be converting them gradually to WebKit style. Please do this for all other arguments of this method as well.

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