Bug 116821 - isDisabledFormControl() should apply for shadow dom element also
Summary: isDisabledFormControl() should apply for shadow dom element also
Status: UNCONFIRMED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-27 04:36 PDT by Santosh Mahto
Modified: 2013-05-28 09:25 PDT (History)
12 users (show)

See Also:


Attachments
Patch (1.17 KB, patch)
2013-05-27 04:43 PDT, Santosh Mahto
benjamin: review-
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (500.26 KB, application/zip)
2013-05-27 11:38 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Santosh Mahto 2013-05-27 04:36:38 PDT
Currently isDisabledFormControl() check for all form control element whether it is disabled or not
so for <select disable> </select> it will return true.

But is doesnot  apply to any shadow DOM element that are part of form control element 

 <Input disable>  </input>  --> isDisabledFormControl()  return true;
 but for  all shadow Nodes of input element  (TextControlInnerTextElement, SpinButtonElement)   it return false.

So If ShadowHost Node (form control) is disabled then isDisabledFormControl()  should return true for Host and also for Shadow Nodes.


consequence of this is that  shadow Nodes of Form control still receives mouse event while mouse event for Form Control element  is disabled
Comment 1 Santosh Mahto 2013-05-27 04:43:47 PDT
Created attachment 202971 [details]
Patch
Comment 2 Build Bot 2013-05-27 11:38:38 PDT
Comment on attachment 202971 [details]
Patch

Attachment 202971 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/725120

New failing tests:
fast/forms/input-readonly-select.html
fast/forms/disabled-mousedown-event.html
Comment 3 Build Bot 2013-05-27 11:38:39 PDT
Created attachment 202995 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3
Comment 4 Benjamin Poulain 2013-05-27 23:03:53 PDT
Comment on attachment 202971 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=202971&action=review

Your patch needs a test, a description in the ChangeLog.
It looks like it also broke some tests. Make sure you run the tests before submitting something.

> Source/WebCore/dom/Element.cpp:1142
> +    Element* host = shadowHost();
> +    if (host)
> +        return host->isDisabledFormControl();

Following WebKit's style, this would be: 
if (Element* host = shadowHost())
    return host->isDisabledFormControl();
Comment 5 Benjamin Poulain 2013-05-27 23:06:12 PDT
Until we re-enable web components, shadow dom is pretty much internal.
It would be good to explain why this change is needed.

I guess a shadow tree is visible for a form element, the state "disabled" changes from JavaScript, and the tree above is still usable?
Comment 6 Santosh Mahto 2013-05-27 23:50:54 PDT
(In reply to comment #5)
> Until we re-enable web components, shadow dom is pretty much internal.
> It would be good to explain why this change is needed.
    the change is not final, first i  need review comments to validate the points.

> I guess a shadow tree is visible for a form element, the state "disabled" changes from JavaScript, and the tree above is still usable? 
   
the bug  already here that is related to this is https://bugs.webkit.org/show_bug.cgi?id=102841, 
even I am not sure of exact behaviour but I think that if form element  is disabled then all of associated  shadow dom nodes should not receive mouse event

In webkit we dont dispatch mouse event to node if it is form element and disabled. I think the behaviour should extend for shadow dom node.

I need  more views on my points.
adding appropriate guy would certainly help in deciding 

and the tree above is still usable?--> No  because host>isDisabledFormControl();


Sorry for test failure as I am not mac man