[Webkit-unassigned] [Bug 27048] [Gtk] Implement STATE_FOCUSED, STATE_FOCUSABLE, and corresponding events for text objects

[bugzilla-daemon at webkit.org]

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


Mario Sanchez Prada <msanchez at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #74224|0                           |1
        is obsolete|                            |
  Attachment #76411|                            |review?
               Flag|                            |




--- Comment #9 from Mario Sanchez Prada <msanchez at igalia.com>  2010-12-13 11:13:50 PST ---
Created an attachment (id=76411)
 --> (https://bugs.webkit.org/attachment.cgi?id=76411&action=review)
Patch proposal + Layout Test

Attaching patch addressing all those issues. See details below...

(In reply to comment #8)
> (From update of attachment 74224 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=74224&action=review
> 
> Mostly looks good to me, just have some doubts/comments, see below.
> 
> > WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:506
> > +    Settings* settings = coreObject->document()->frame()->settings();
> 
> Some of those can be in some situations NULL. At the very least frame(). If this function can never be called in those cases then you should ASSERT.

Fixed, by adding extra checks to the function (better to be safe!)

> > WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:521
> > +    return !selectionBelongsToObject(coreObject, selection) || selection.isNone() ? 0 : 1;
> 
> How is this grouping exactly? It's a quite confusing statement.

Fixed by replacing it for equivalent code.

> > WebCore/editing/gtk/SelectionControllerGtk.cpp:48
> > +    static RefPtr<AccessibilityObject> oldObject = 0;
> 
> Mmm, I think you want to use DEFINE_STATIC_LOCAL here?

Fixed.

> > WebCore/editing/gtk/SelectionControllerGtk.cpp:82
> > +    // Reset lastFocuseNode and return for no valid selections.
> 
> I don't see that reset happening?

I meant resetting it to 'zero'. Fixed comment

Now asking for review again then...

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