[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 #61783|0                           |1
        is obsolete|                            |
  Attachment #61861|                            |review?
               Flag|                            |




--- Comment #27 from Mario Sanchez Prada <msanchez at igalia.com>  2010-07-16 16:49:32 PST ---
Created an attachment (id=61861)
 --> (https://bugs.webkit.org/attachment.cgi?id=61861)
3. Notify accessibility when something changes in a text node

Now attaching a new patch to replace this one as well...

(In reply to comment #24)
> (From update of attachment 61783 [details])
> Adding r=me. I think the changes you need to make are straightforward for this one. I think calling these methods can be mostly reduce to two lines for each instance
> if (AXObjectCache::)
>     document()->axCache()->nodeTextChange()

As I said in my previous comment, I'm not a committer so if you're ok with this new patch I'd appreciate that, if possible, you set the commit-queue+ flag alongside with the review+ (supposing it's correct!).

> WebCore/editing/AppendNodeCommand.cpp:60
>  +                                                                    0, len);
> indentation

Fixed, alongside with the rest of the indentation issues.

> WebCore/editing/AppendNodeCommand.cpp:61
>  +          }
> do you need the line breaking here in nodeTextChangeNotification?

I'm very used not to trespass the 80 line whenever possible so that's why I broke it. Anyway, I was checking other files and it seems in WK code it's not that strange to find long lines every now and then, so I've just changed this to one line as well.

> WebCore/editing/AppendNodeCommand.cpp:78
>  +          }
> I think the len > 0, node->nodeValue() != "\n" should go into the logic inside AXObjectCache. Different platforms may want to process those, so i think it should be left up to AX code to handle those cases.
> it will also make these code snippets much simpler

Absolutely. The length check should be in the platform specific code (and it actually is already there!), and doing it outside it's just plain useless and unneeded.

However, I need to leave the "\n" check in AppendNodeCommand since that's there to avoid to notify a11y twice, as in that case it happens that the InserNodeBeforeCommand will be always executed whenever a linebreak is done (while AppendNodecommand is only run when making a linebreak at the end of the text), so there's no need to do it also here (and it would even be a mistake IMHO)

> WebCore/editing/DeleteFromTextNodeCommand.cpp:63
>  +  
> for one line if statements, no braces needed

Done.

> WebCore/editing/DeleteFromTextNodeCommand.cpp:82
>  +  }
> no braces needed

Done

> WebCore/editing/InsertIntoTextNodeCommand.cpp:57
>  +      }
> no braces needed

Done

> WebCore/editing/InsertIntoTextNodeCommand.cpp:71
>  +  
> no braces needed

Done

> 
> WebCore/editing/InsertNodeBeforeCommand.cpp:62
>  +          }
> same comments, let the AX code handle whether it should process this based on length or not
> 
> WebCore/editing/InsertNodeBeforeCommand.cpp:78
>  +          }
> ditto as above
> 

Done

> WebCore/editing/InsertNodeBeforeCommand.cpp:84
>  +  1.7.0.4
> extra line removed, should not be

As I said in the previous command, I don't understand this thing :-)

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