Bug 122968 - [ATK] Use atk_object_notify_state_change instead of manually emitting signals
Summary: [ATK] Use atk_object_notify_state_change instead of manually emitting signals
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Anton Obzhirov
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-10-17 09:55 PDT by Mario Sanchez Prada
Modified: 2013-10-21 07:00 PDT (History)
8 users (show)

See Also:

Attachments
Patch (5.88 KB, patch)
2013-10-18 03:09 PDT, Anton Obzhirov 		no flags Details | Formatted Diff | Diff
Patch (5.91 KB, patch)
2013-10-21 01:53 PDT, Anton Obzhirov 		no flags Details | Formatted Diff | Diff
Patch (7.99 KB, patch)
2013-10-21 06:46 PDT, Anton Obzhirov 		mario: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mario Sanchez Prada 2013-10-17 09:55:27 PDT 
Using atk_object_notify_state_change() instead of manually calling g_signal_emit_by_name() has the clear benefit that we don't emit by mistake an incorrect change of state, since we are forced to use values from the AtkState enum instead of raw strings.

We should change it in AXObjectCacheAtk.cpp to avoid further confusion.

[1]https://developer.gnome.org/atk/stable/AtkObject.html#atk-object-notify-state-change
Comment 1 Radar WebKit Bug Importer 2013-10-17 09:56:01 PDT 
<rdar://problem/15251851>
Comment 2 Anton Obzhirov 2013-10-17 09:58:28 PDT 
I'll do refactoring for this one.
Comment 3 Anton Obzhirov 2013-10-18 03:09:26 PDT 
Created attachment 214553 [details]
Patch
Comment 4 Anton Obzhirov 2013-10-21 01:53:11 PDT 
Created attachment 214715 [details]
Patch
Comment 5 Mario Sanchez Prada 2013-10-21 05:30:17 PDT 
Comment on attachment 214715 [details]
Patch

Lgtm as a partial patch, but you still need to change a few more places:

  accessibility/atk/WebKitAccessibleWrapperAtk.cpp
  editing/atk/FrameSelectionAtk.cpp
Comment 6 Anton Obzhirov 2013-10-21 05:34:24 PDT 
(In reply to comment #5)
> (From update of attachment 214715 [details])
> Lgtm as a partial patch, but you still need to change a few more places:
> 
>   accessibility/atk/WebKitAccessibleWrapperAtk.cpp
>   editing/atk/FrameSelectionAtk.cpp

Thanks, will do now.
Comment 7 Anton Obzhirov 2013-10-21 06:46:34 PDT 
Created attachment 214734 [details]
Patch
Comment 8 Mario Sanchez Prada 2013-10-21 06:57:38 PDT 
Comment on attachment 214734 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=214734&action=review

lgtm

> Source/WebCore/ChangeLog:3
> +        [ATK] Use atk_object_notify_state_change instead of manuall emitting signals

manuall -> manually

Anyway, that was my fault while reporting the bug, so I will push it manually for you and fix that before landing
Comment 9 Mario Sanchez Prada 2013-10-21 07:00:31 PDT 
Committed r157718: <http://trac.webkit.org/changeset/157718>