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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Sep 7 14:52:35 PDT 2010


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.

More information about the webkit-unassigned mailing list