[Webkit-unassigned] [Bug 25898] [Gtk] object:text-changed events should be emitted for entries and password text

[bugzilla-daemon at webkit.org]

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


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

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #61782|0                           |1
        is obsolete|                            |
  Attachment #61859|                            |review?
               Flag|                            |




--- Comment #26 from Mario Sanchez Prada <msanchez at igalia.com>  2010-07-16 16:36:35 PST ---
Created an attachment (id=61859)
 --> (https://bugs.webkit.org/attachment.cgi?id=61859)
2. New functions in AXObjectCache to call when text changes in a node

First of all, thanks for reviewing and sorry for marking the patch you set as review+ as obsolete, but I had to do it since I couldn't commit the changes by myself anyway (I'm not committer). So I'm now just replacing the old version of the patch with a new one so you could review it and, if it's correct, both mark it as review+ and commit-queue+ :-)

(In reply to comment #23)
> (From update of attachment 61782 [details])
> This patch is just about there. I'm putting review- just because i'd like to see it one more time before it goes through. thanks!
> 
> WebCore/accessibility/AXObjectCache.cpp:473
>  +  }
> it looks like you don't really need this method, since its only used by the RenderObject version.

Sure, you're right. I did it that way following the lead of the AXObject::postNotification() method, thinking that perhaps that was the way to go because of some yet-unknown reason to me... but I normally think needed methods should get in, and if you think the same that's a good reason for me to remove it.

So... done.

> WebCore/accessibility/gtk/AXObjectCacheAtk.cpp:68
>  +  
> This looks like a roundabout way to get object->parentObject()->wrapper(). 

True. I'm still getting used to this forest of trees in WK and sometimes I get a bit lost... as in this case :-). Thanks for pointing it out, I've tried your suggestion and works also as a charm (but cleaner).

Done.

> WebCore/accessibility/gtk/AXObjectCacheAtk.cpp:73
>  +  
> identation is wrong
> 
> WebCore/accessibility/gtk/AXObjectCacheAtk.cpp:88
>  +  
> identation is wrong

Fixed all indentation issues in all files.

> WebCore/accessibility/gtk/AXObjectCacheAtk.cpp:94
>  +  }
> looks like you should use a switch statement here which sets a char* or String to "text-change*". then you'd only have to call g_signal* one time.

Done (I've used a GOwnPtr to avoid using g_free(), hope that's the right thing to do)

> WebCore/accessibility/win/AXObjectCacheWin.cpp:113
>  +  1.7.0.4
> extra line removal not needed

Not sure I understand this one... Isn't it just the end of the patch file?

> WebCore/accessibility/gtk/AXObjectCacheAtk.cpp:65
>  +  
> identation

Fixed.

Thanks for the review. As I said at the beginning here you have attached a new patch addressing these issues

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