[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 #8 from Dimitri Glazkov (Google) <dglazkov at chromium.org>  2012-05-16 13:06:53 PST ---
(From update of attachment 142294)
View in context: https://bugs.webkit.org/attachment.cgi?id=142294&action=review

>>> Source/WebCore/html/shadow/TextFieldDecorationElement.cpp:143
>>> +        style->setVisibility(HIDDEN);
>> 
>> If you need this code, something has gone horribly wrong.
> 
> So I don't understand the distinction between styles set by SetInlineStyleProperty and those set via RenderStyle, but just setting the inline style (either visibility:hidden or display:none) will not cause the element to disappear. As far as I can tell it has to be set here, and propagating from the inline style just seemed like the easiest way to accomplish this. Like I said, I don't understand why because I don't understand the distinction between them, but it's how it works at the moment.
> 
> The second line is because currently if the HTMLInputElement is hidden, the decorator still shows up. This doesn't seem like it should happen either.

When you set inline style on an element, the style recalculation will be triggered and the correct style should be created for TextFieldDecorationElement. Unfortunately, the customStyleForRenderer method short-circuits this process and forces you to do gnarly hacks like this. The problem with not fixing this is that _every_ time you need to modify a style on the TextFieldDecorationElement, you will have to add a little condition just like this to customStyleForRenderer. That's just bad design. Not your design, but still unacceptable. Kent-san, can we please change this to something that is not so rigid?

>>> Source/WebKit/chromium/public/WebStyledElement.h:32
>>> +#define WebStyledElement_h
>> 
>> As I mentioned before, it's not the right approach. Exposing StyledElement in WebKit API is an overkill for this solution.
>> 
>> Perhaps instead we could morph the API to be a list of decorators in a document?
>> 
>> Then, we can listen to normal document events and react to them by iterating through the decorators and operating on them. In other words, instead of exposing WebStyledElement, expose the WebTextFieldDecorator? WDYT?
> 
> I'm not sure I understand what you are suggesting. Exposing WebTextFieldDecorator instead of WebStyledElement seems fine to me, though we can do that by just changing the function definition of decorationAdded(). It seems like you are suggesting something larger. How exactly are you proposing to change WebTextFieldDecoratorClient?

The design that I am proposing is:

* Adding textfield decorators should be limited to simply initializing the shadow DOM subtrees with hidden elements. There are no decisions made about visibility during this, since it's too early to know the context (for your particular case).
* We register a "load" or "DOMContentLoad" event listener
* When the event fires, we iterate over all decorators and then make our decisions on whether the decorator should be visibile
* WebTextFieldDecorator has a setVisible(bool) method, which we set when we make our decision

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.

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