[Webkit-unassigned] [Bug 86557] Allow WebTextFieldDecoratorClient to see applied decorations.

[bugzilla-daemon at webkit.org]

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





--- Comment #14 from Garrett Casto <gcasto at chromium.org>  2012-05-17 14:05:22 PST ---
(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #10)
> > > (In reply to comment #9)
> > > > 
> > > > So the second and third point here suggest to me something where ChromeClientImpl asks Chromium if a particular element should be made visible (when the event handler fires). The last point implies that Chromium can change the visibility of the decorator whenever. I'm not sure what I'm missing, but I do think that the Chromium code needs to be able to change the visibility of a decorator at arbitrary times, since in my case the client is not going to know if a decorator should be visible until it gets some information back from the browser process. Assuming we change WebStyledElement -> WebTextFieldDecorator and fix the style propagation issue, the current WebTextFieldDecoratorClient works fine for me. Do you not approve of the interface change to WebTextFieldDecoratorClient, or was there something else you were hoping to get out of this new design?
> > > >
> > > 
> > > Ah, I see. I've forgotten about the IPC thing. So you need to hold on to WebTextFieldDecorator until you get the data back from the browser process? When would you request this data? addTextFieldDecorationsTo is too early, right?
> > > 
> > 
> > The request is actually already being made for autofill purposes. It happens in didFinishDocumentLoad, and I don't think that this would need to move. As you mentioned, addTextFieldDecorationsTo is too early for this as we don't have the complete form data.
> 
> Great, so this makes the API we need fairly simple: 
> 1) a way to iterate through all elements per decorator in a document and;
> 2) a way to make some of them visible.
> 

I mentioned this before, but I think that the API that I've currently proposed (modulo WebStyledElement -> WebTextFieldDecorator and the style propagation problems) allows this, though the user does have to worry about the lifetimes of the Elements. Is the lifetime issue what you are worried about, or just general cleanliness of the API, or what? Are you proposing adding function to WebView to iterate through these elements?

Assuming that you don't want the client to worry about lifetimes, there are two other issues. The first is that having a way to do lookups as well as iteration through these elements would be useful. I'm thinking in particular of when we get the IPC from the browser iterating through all of the decorated elements looking for the one that we want to show seems slightly silly. Relatedly, I'm actually not sure if we need iteration, as I currently accomplish this by just iterating through all the forms on a page and I'm not sure how much faster just iterating through all the decorated elements would be. Secondly, we would need to change handleClick() to include a WebTextFieldDecorator, since I need that to determine how to center the popup. Not a big change though.

I would also like to say that I would prefer that we get _something_ checked in reasonably soon that works. I'm willing to cleanup the API, I would just like to be able to have a testable UI in Chromium sooner rather than later.

> > 
> > > > > If we need to do something more complex, like reacting when an input element is added or removed from the document tree, you need an accurate signal of when that happens. You can't rely on Chrome::addTextFieldDecorationsTo, since this will be called way too many times (and for most of them, not when you need it). Perhaps a MutationObserver would be useful here.
> > > > 
> > > > I do not think that I need anything more complex. It's possible that eventually I would want this to be able to deal with dynamically generated forms, but first the password manager will need to deal with them.
> > > 
> > > Great! :)

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