Bug 122968

Summary: [ATK] Use atk_object_notify_state_change instead of manually emitting signals
Product: WebKit Reporter: Mario Sanchez Prada <mario>
Component: AccessibilityAssignee: Anton Obzhirov <obzhirov>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, jdiggs, obzhirov, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch mario: review+

Mario Sanchez Prada
Reported 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
Attachments
Patch (5.88 KB, patch)
2013-10-18 03:09 PDT, Anton Obzhirov 		no flags
DetailsFormatted DiffDiff
Patch (5.91 KB, patch)
2013-10-21 01:53 PDT, Anton Obzhirov 		no flags
DetailsFormatted DiffDiff
Patch (7.99 KB, patch)
2013-10-21 06:46 PDT, Anton Obzhirov 		mario: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2013-10-17 09:56:01 PDT
<rdar://problem/15251851>
Anton Obzhirov
Comment 2 2013-10-17 09:58:28 PDT
I'll do refactoring for this one.
Anton Obzhirov
Comment 3 2013-10-18 03:09:26 PDT
Created attachment 214553 [details] Patch
Anton Obzhirov
Comment 4 2013-10-21 01:53:11 PDT
Created attachment 214715 [details] Patch
Mario Sanchez Prada
Comment 5 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
Anton Obzhirov
Comment 6 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.
Anton Obzhirov
Comment 7 2013-10-21 06:46:34 PDT
Created attachment 214734 [details] Patch
Mario Sanchez Prada
Comment 8 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
Mario Sanchez Prada
Comment 9 2013-10-21 07:00:31 PDT
Committed r157718: <http://trac.webkit.org/changeset/157718>
Note You need to log in before you can comment on or make changes to this bug.