[Webkit-unassigned] [Bug 132527] AX: [ATK] [PATCH] add text-caret-moved signal to accessibilityController

[bugzilla-daemon at webkit.org]

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





--- Comment #16 from Jarek Czekalski <jarekczek at poczta.onet.pl>  2014-05-29 09:38:14 PST ---
(From update of attachment 231629)
View in context: https://bugs.webkit.org/attachment.cgi?id=231629&action=review

>> Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityNotificationHandlerAtk.cpp:150
>> +                0, lastArgument + 1 - 1, arguments + 1, 0);
> 
> lastArgument + 1 - 1? That looks like a strange idiom, even if it's correct. I personally think something like this would be less cryptic:
> 
>  [...]
> 
>  size_t numOfExtraArgs = extraArgs.size();
>  for (int i = 0; i < numOfExtraArgs; i++)
>      arguments[i + 2] = extraArgs[i];
> 
>  if (elementNotificationHandler != notificationHandlers.end()) {
>      // Listener for one element. As arguments, it gets the notification name
>      // plus any extra argument that must be needed to pass to the callback
>      JSObjectCallAsFunction(jsContext,
>          const_cast<JSObjectRef>(elementNotificationHandler->value->notificationFunctionCallback()),
>          0, numOfExtraArgs + 1, arguments + 1, 0);
> 
>  [...]
> 
> 
> What do you think?

I think this is a good change. The comments were outdated indeed. Have a look at them once more before committing, because I rewrote them.

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