From 46d18446589d5e6824a5d60d77c8778d3535ce67 Mon Sep 17 00:00:00 2001 From: Fedor Date: Thu, 12 Mar 2020 20:41:01 +0300 Subject: [PATCH] Remove -moz-user-input disabled to improve event handling. --- devtools/client/themes/webconsole.css | 1 - .../shared/css/generated/properties-db.js | 2 -- dom/html/HTMLLabelElement.h | 2 -- dom/html/HTMLObjectElement.h | 2 -- dom/html/HTMLOptGroupElement.cpp | 11 +++++----- dom/html/HTMLOptGroupElement.h | 4 ---- dom/html/HTMLOptionElement.h | 4 ---- dom/html/HTMLOutputElement.h | 2 -- dom/html/nsGenericHTMLElement.cpp | 21 ++++++------------- dom/html/nsGenericHTMLElement.h | 6 ++---- dom/interfaces/base/nsIDOMWindowUtils.idl | 2 +- layout/base/nsCaret.cpp | 5 ++--- layout/forms/nsComboboxControlFrame.cpp | 4 +--- layout/forms/nsFormControlFrame.cpp | 6 ++---- layout/forms/nsGfxButtonControlFrame.cpp | 5 +---- layout/forms/nsImageControlFrame.cpp | 8 +------ layout/forms/nsListControlFrame.cpp | 11 +++------- layout/generic/nsFrame.cpp | 13 ++++++++++++ layout/generic/nsIFrame.h | 5 +++++ layout/style/nsCSSProps.cpp | 2 -- layout/style/nsStyleConsts.h | 2 -- layout/style/res/forms.css | 1 - layout/style/test/property_database.js | 2 +- testing/web-platform/meta/MANIFEST.json | 4 ++++ .../dom/events/event-disabled-dynamic.html | 21 +++++++++++++++++++ 25 files changed, 68 insertions(+), 78 deletions(-) create mode 100644 testing/web-platform/tests/dom/events/event-disabled-dynamic.html diff --git a/devtools/client/themes/webconsole.css b/devtools/client/themes/webconsole.css index 89bb38628..85c73264c 100644 --- a/devtools/client/themes/webconsole.css +++ b/devtools/client/themes/webconsole.css @@ -14,7 +14,6 @@ a { -moz-user-focus: normal; - -moz-user-input: enabled; cursor: pointer; text-decoration: underline; } diff --git a/devtools/shared/css/generated/properties-db.js b/devtools/shared/css/generated/properties-db.js index 25d9e2d33..eda2c7148 100644 --- a/devtools/shared/css/generated/properties-db.js +++ b/devtools/shared/css/generated/properties-db.js @@ -1440,8 +1440,6 @@ exports.CSS_PROPERTIES = { "supports": [], "values": [ "auto", - "disabled", - "enabled", "inherit", "initial", "none", diff --git a/dom/html/HTMLLabelElement.h b/dom/html/HTMLLabelElement.h index c8385fc53..4057ffef6 100644 --- a/dom/html/HTMLLabelElement.h +++ b/dom/html/HTMLLabelElement.h @@ -59,8 +59,6 @@ public: using nsGenericHTMLElement::Focus; virtual void Focus(mozilla::ErrorResult& aError) override; - virtual bool IsDisabled() const override { return false; } - // nsIContent virtual nsresult PostHandleEvent( EventChainPostVisitor& aVisitor) override; diff --git a/dom/html/HTMLObjectElement.h b/dom/html/HTMLObjectElement.h index 4041b78a3..5226154da 100644 --- a/dom/html/HTMLObjectElement.h +++ b/dom/html/HTMLObjectElement.h @@ -77,8 +77,6 @@ public: NS_IMETHOD Reset() override; NS_IMETHOD SubmitNamesValues(HTMLFormSubmission *aFormSubmission) override; - virtual bool IsDisabled() const override { return false; } - virtual void DoneAddingChildren(bool aHaveNotified) override; virtual bool IsDoneAddingChildren() override; diff --git a/dom/html/HTMLOptGroupElement.cpp b/dom/html/HTMLOptGroupElement.cpp index 8a044fbf3..9e738961d 100644 --- a/dom/html/HTMLOptGroupElement.cpp +++ b/dom/html/HTMLOptGroupElement.cpp @@ -53,15 +53,14 @@ HTMLOptGroupElement::PreHandleEvent(EventChainPreVisitor& aVisitor) aVisitor.mCanHandle = false; // Do not process any DOM events if the element is disabled // XXXsmaug This is not the right thing to do. But what is? - if (HasAttr(kNameSpaceID_None, nsGkAtoms::disabled)) { + if (IsDisabled()) { return NS_OK; } - nsIFrame* frame = GetPrimaryFrame(); - if (frame) { - const nsStyleUserInterface* uiStyle = frame->StyleUserInterface(); - if (uiStyle->mUserInput == StyleUserInput::None || - uiStyle->mUserInput == StyleUserInput::Disabled) { + if (nsIFrame* frame = GetPrimaryFrame()) { + // FIXME(emilio): This poking at the style of the frame is broken unless we + // flush before every event handling, which we don't really want to. + if (frame->StyleUserInterface()->mUserInput == StyleUserInput::None) { return NS_OK; } } diff --git a/dom/html/HTMLOptGroupElement.h b/dom/html/HTMLOptGroupElement.h index d53a2e32b..e46a6a953 100644 --- a/dom/html/HTMLOptGroupElement.h +++ b/dom/html/HTMLOptGroupElement.h @@ -46,10 +46,6 @@ public: virtual nsIDOMNode* AsDOMNode() override { return this; } - virtual bool IsDisabled() const override { - return HasAttr(kNameSpaceID_None, nsGkAtoms::disabled); - } - bool Disabled() const { return GetBoolAttr(nsGkAtoms::disabled); diff --git a/dom/html/HTMLOptionElement.h b/dom/html/HTMLOptionElement.h index e220b84df..4b5e192ff 100644 --- a/dom/html/HTMLOptionElement.h +++ b/dom/html/HTMLOptionElement.h @@ -67,10 +67,6 @@ public: nsresult CopyInnerTo(mozilla::dom::Element* aDest); - virtual bool IsDisabled() const override { - return HasAttr(kNameSpaceID_None, nsGkAtoms::disabled); - } - bool Disabled() const { return GetBoolAttr(nsGkAtoms::disabled); diff --git a/dom/html/HTMLOutputElement.h b/dom/html/HTMLOutputElement.h index 588262480..6b6c3f66c 100644 --- a/dom/html/HTMLOutputElement.h +++ b/dom/html/HTMLOutputElement.h @@ -35,8 +35,6 @@ public: NS_IMETHOD Reset() override; NS_IMETHOD SubmitNamesValues(HTMLFormSubmission* aFormSubmission) override; - virtual bool IsDisabled() const override { return false; } - nsresult Clone(mozilla::dom::NodeInfo* aNodeInfo, nsINode** aResult) const override; bool ParseAttribute(int32_t aNamespaceID, nsIAtom* aAttribute, diff --git a/dom/html/nsGenericHTMLElement.cpp b/dom/html/nsGenericHTMLElement.cpp index 2f890325a..0c8bcc9c8 100644 --- a/dom/html/nsGenericHTMLElement.cpp +++ b/dom/html/nsGenericHTMLElement.cpp @@ -2109,14 +2109,6 @@ nsGenericHTMLFormElement::PreHandleEvent(EventChainPreVisitor& aVisitor) return nsGenericHTMLElement::PreHandleEvent(aVisitor); } -/* virtual */ -bool -nsGenericHTMLFormElement::IsDisabled() const -{ - return HasAttr(kNameSpaceID_None, nsGkAtoms::disabled) || - (mFieldSet && mFieldSet->IsDisabled()); -} - void nsGenericHTMLFormElement::ForgetFieldSet(nsIContent* aFieldset) { @@ -2308,14 +2300,13 @@ nsGenericHTMLFormElement::IsElementDisabledForEvents(EventMessage aMessage, break; } - bool disabled = IsDisabled(); - if (!disabled && aFrame) { - const nsStyleUserInterface* uiStyle = aFrame->StyleUserInterface(); - disabled = uiStyle->mUserInput == StyleUserInput::None || - uiStyle->mUserInput == StyleUserInput::Disabled; - + // FIXME(emilio): This poking at the style of the frame is slightly bogus + // unless we flush before every event, which we don't really want to do. + if (aFrame && + aFrame->StyleUserInterface()->mUserInput == StyleUserInput::None) { + return true; } - return disabled; + return IsDisabled(); } void diff --git a/dom/html/nsGenericHTMLElement.h b/dom/html/nsGenericHTMLElement.h index 72039f266..acbebe087 100644 --- a/dom/html/nsGenericHTMLElement.h +++ b/dom/html/nsGenericHTMLElement.h @@ -817,8 +817,8 @@ public: /** * Returns the current disabled state of the element. */ - virtual bool IsDisabled() const { - return false; + bool IsDisabled() const { + return HasAttr(kNameSpaceID_None, nsGkAtoms::disabled); } bool IsHidden() const @@ -1222,8 +1222,6 @@ public: virtual nsresult PreHandleEvent( mozilla::EventChainPreVisitor& aVisitor) override; - virtual bool IsDisabled() const override; - /** * This callback is called by a fieldest on all its elements whenever its * disabled attribute is changed so the element knows its disabled state diff --git a/dom/interfaces/base/nsIDOMWindowUtils.idl b/dom/interfaces/base/nsIDOMWindowUtils.idl index 70ec7e0ae..fcfe407e8 100644 --- a/dom/interfaces/base/nsIDOMWindowUtils.idl +++ b/dom/interfaces/base/nsIDOMWindowUtils.idl @@ -1786,7 +1786,7 @@ interface nsIDOMWindowUtils : nsISupports { /** * In certain cases the event handling of nodes, form controls in practice, * may be disabled. Such cases are for example the existence of disabled - * attribute or -moz-user-input: none/disabled. + * attribute or -moz-user-input: none. */ boolean isNodeDisabledForEvents(in nsIDOMNode aNode); diff --git a/layout/base/nsCaret.cpp b/layout/base/nsCaret.cpp index eca22f3ba..8396726c6 100644 --- a/layout/base/nsCaret.cpp +++ b/layout/base/nsCaret.cpp @@ -511,7 +511,7 @@ nsCaret::GetPaintGeometry(nsRect* aRect) CheckSelectionLanguageChange(); int32_t frameOffset; - nsIFrame *frame = GetFrameAndOffset(GetSelectionInternal(), + nsIFrame* frame = GetFrameAndOffset(GetSelectionInternal(), mOverrideContent, mOverrideOffset, &frameOffset); if (!frame) { return nullptr; @@ -521,8 +521,7 @@ nsCaret::GetPaintGeometry(nsRect* aRect) const nsStyleUserInterface* userinterface = frame->StyleUserInterface(); if ((!mIgnoreUserModify && userinterface->mUserModify == StyleUserModify::ReadOnly) || - userinterface->mUserInput == StyleUserInput::None || - userinterface->mUserInput == StyleUserInput::Disabled) { + frame->IsContentDisabled()){ return nullptr; } diff --git a/layout/forms/nsComboboxControlFrame.cpp b/layout/forms/nsComboboxControlFrame.cpp index 5a9438939..78185616f 100644 --- a/layout/forms/nsComboboxControlFrame.cpp +++ b/layout/forms/nsComboboxControlFrame.cpp @@ -1165,9 +1165,7 @@ nsComboboxControlFrame::HandleEvent(nsPresContext* aPresContext, // If we have style that affects how we are selected, feed event down to // nsFrame::HandleEvent so that selection takes place when appropriate. - const nsStyleUserInterface* uiStyle = StyleUserInterface(); - if (uiStyle->mUserInput == StyleUserInput::None || - uiStyle->mUserInput == StyleUserInput::Disabled) { + if (IsContentDisabled()) { return nsBlockFrame::HandleEvent(aPresContext, aEvent, aEventStatus); } return NS_OK; diff --git a/layout/forms/nsFormControlFrame.cpp b/layout/forms/nsFormControlFrame.cpp index 4ee62acbf..8dbe564dd 100644 --- a/layout/forms/nsFormControlFrame.cpp +++ b/layout/forms/nsFormControlFrame.cpp @@ -183,10 +183,8 @@ nsFormControlFrame::HandleEvent(nsPresContext* aPresContext, WidgetGUIEvent* aEvent, nsEventStatus* aEventStatus) { - // Check for user-input:none style - const nsStyleUserInterface* uiStyle = StyleUserInterface(); - if (uiStyle->mUserInput == StyleUserInput::None || - uiStyle->mUserInput == StyleUserInput::Disabled) { + // Check for disabled content so that selection works properly (?). + if (IsContentDisabled()) { return nsFrame::HandleEvent(aPresContext, aEvent, aEventStatus); } return NS_OK; diff --git a/layout/forms/nsGfxButtonControlFrame.cpp b/layout/forms/nsGfxButtonControlFrame.cpp index 90da437f7..393145e0b 100644 --- a/layout/forms/nsGfxButtonControlFrame.cpp +++ b/layout/forms/nsGfxButtonControlFrame.cpp @@ -227,10 +227,7 @@ nsGfxButtonControlFrame::HandleEvent(nsPresContext* aPresContext, // from being called. The nsFrame::HandleEvent causes the button label // to be selected (Drawn with an XOR rectangle over the label) - // do we have user-input style? - const nsStyleUserInterface* uiStyle = StyleUserInterface(); - if (uiStyle->mUserInput == StyleUserInput::None || - uiStyle->mUserInput == StyleUserInput::Disabled) { + if (IsContentDisabled()) { return nsFrame::HandleEvent(aPresContext, aEvent, aEventStatus); } return NS_OK; diff --git a/layout/forms/nsImageControlFrame.cpp b/layout/forms/nsImageControlFrame.cpp index 212fa9356..8aef41538 100644 --- a/layout/forms/nsImageControlFrame.cpp +++ b/layout/forms/nsImageControlFrame.cpp @@ -150,15 +150,9 @@ nsImageControlFrame::HandleEvent(nsPresContext* aPresContext, return NS_OK; } - // do we have user-input style? - const nsStyleUserInterface* uiStyle = StyleUserInterface(); - if (uiStyle->mUserInput == StyleUserInput::None || - uiStyle->mUserInput == StyleUserInput::Disabled) { + if (IsContentDisabled()) { return nsFrame::HandleEvent(aPresContext, aEvent, aEventStatus); } - if (mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::disabled)) { // XXX cache disabled - return NS_OK; - } *aEventStatus = nsEventStatus_eIgnore; diff --git a/layout/forms/nsListControlFrame.cpp b/layout/forms/nsListControlFrame.cpp index cc5f37f9a..58e81039f 100644 --- a/layout/forms/nsListControlFrame.cpp +++ b/layout/forms/nsListControlFrame.cpp @@ -920,16 +920,11 @@ nsListControlFrame::HandleEvent(nsPresContext* aPresContext, if (nsEventStatus_eConsumeNoDefault == *aEventStatus) return NS_OK; - // do we have style that affects how we are selected? - // do we have user-input style? - const nsStyleUserInterface* uiStyle = StyleUserInterface(); - if (uiStyle->mUserInput == StyleUserInput::None || - uiStyle->mUserInput == StyleUserInput::Disabled) { + // disabled state affects how we're selected, but we don't want to go through + // nsHTMLScrollFrame if we're disabled. + if (IsContentDisabled()) { return nsFrame::HandleEvent(aPresContext, aEvent, aEventStatus); } - EventStates eventStates = mContent->AsElement()->State(); - if (eventStates.HasState(NS_EVENT_STATE_DISABLED)) - return NS_OK; return nsHTMLScrollFrame::HandleEvent(aPresContext, aEvent, aEventStatus); } diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp index bbbb5c332..8d4ea8754 100644 --- a/layout/generic/nsFrame.cpp +++ b/layout/generic/nsFrame.cpp @@ -5557,6 +5557,19 @@ nsFrame::Reflow(nsPresContext* aPresContext, NS_FRAME_SET_TRUNCATION(aStatus, aReflowInput, aDesiredSize); } +bool +nsIFrame::IsContentDisabled() const +{ + // FIXME(emilio): Doing this via CSS means callers must ensure the style is up + // to date, and they don't! + if (StyleUserInterface()->mUserInput == StyleUserInput::None) { + return true; + } + + auto* element = nsGenericHTMLElement::FromContentOrNull(GetContent()); + return element && element->IsDisabled(); +} + nsresult nsFrame::CharacterDataChanged(CharacterDataChangeInfo* aInfo) { diff --git a/layout/generic/nsIFrame.h b/layout/generic/nsIFrame.h index 57f5c460c..93eb95099 100644 --- a/layout/generic/nsIFrame.h +++ b/layout/generic/nsIFrame.h @@ -2435,6 +2435,11 @@ public: */ nsIWidget* GetNearestWidget(nsPoint& aOffset) const; + /** + * Whether the content for this frame is disabled, used for event handling. + */ + bool IsContentDisabled() const; + /** * Get the "type" of the frame. May return nullptr. * diff --git a/layout/style/nsCSSProps.cpp b/layout/style/nsCSSProps.cpp index ac2978c27..289a16ecd 100644 --- a/layout/style/nsCSSProps.cpp +++ b/layout/style/nsCSSProps.cpp @@ -2189,8 +2189,6 @@ const KTableEntry nsCSSProps::kUserFocusKTable[] = { const KTableEntry nsCSSProps::kUserInputKTable[] = { { eCSSKeyword_none, StyleUserInput::None }, - { eCSSKeyword_enabled, StyleUserInput::Enabled }, - { eCSSKeyword_disabled, StyleUserInput::Disabled }, { eCSSKeyword_auto, StyleUserInput::Auto }, { eCSSKeyword_UNKNOWN, -1 } }; diff --git a/layout/style/nsStyleConsts.h b/layout/style/nsStyleConsts.h index 6d207aec9..5412eed26 100644 --- a/layout/style/nsStyleConsts.h +++ b/layout/style/nsStyleConsts.h @@ -241,8 +241,6 @@ enum class StyleUserSelect : uint8_t { // user-input enum class StyleUserInput : uint8_t { None, - Enabled, - Disabled, Auto, }; diff --git a/layout/style/res/forms.css b/layout/style/res/forms.css index e7566e183..95025221d 100644 --- a/layout/style/res/forms.css +++ b/layout/style/res/forms.css @@ -425,7 +425,6 @@ optgroup:disabled, select:disabled:disabled /* Need the pseudo-class twice to have the specificity be at least the same as select[size][multiple] above */ { - -moz-user-input: disabled; color: GrayText; background-color: ThreeDLightShadow; cursor: inherit; diff --git a/layout/style/test/property_database.js b/layout/style/test/property_database.js index c75f7b498..3486d15d0 100644 --- a/layout/style/test/property_database.js +++ b/layout/style/test/property_database.js @@ -2203,7 +2203,7 @@ var gCSSProperties = { inherited: true, type: CSS_TYPE_LONGHAND, initial_values: [ "auto" ], - other_values: [ "none", "enabled", "disabled" ], + other_values: [ "none" ], invalid_values: [] }, "-moz-user-modify": { diff --git a/testing/web-platform/meta/MANIFEST.json b/testing/web-platform/meta/MANIFEST.json index 03dda4a1f..4dbbdf43e 100644 --- a/testing/web-platform/meta/MANIFEST.json +++ b/testing/web-platform/meta/MANIFEST.json @@ -15393,6 +15393,10 @@ "path": "dom/events/EventTarget-removeEventListener.html", "url": "/dom/events/EventTarget-removeEventListener.html" }, + { + "path": "dom/events/event-disabled-dynamic.html", + "url": "/dom/events/event-disabled-dynamic.html" + }, { "path": "dom/events/ProgressEvent.html", "url": "/dom/events/ProgressEvent.html" diff --git a/testing/web-platform/tests/dom/events/event-disabled-dynamic.html b/testing/web-platform/tests/dom/events/event-disabled-dynamic.html new file mode 100644 index 000000000..89a3d3b15 --- /dev/null +++ b/testing/web-platform/tests/dom/events/event-disabled-dynamic.html @@ -0,0 +1,21 @@ + + +Test that disabled is honored immediately in presence of dynamic changes + + + + + + + +