Bug 1405087: Don't use -moz-user-input: disabled to decide event handling stuff. r?bz draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 10 Feb 2018 19:34:10 +0100
changeset 755086 b929a416f6b665b3a69cc1fd15d56c6542d5c020
parent 754572 e43f2f6ea111c2d059d95fa9a71516b869a69698
child 755087 3e903e0e4dd519973e3552696010b401859a8f44
push id99101
push userbmo:emilio@crisal.io
push dateWed, 14 Feb 2018 21:16:42 +0000
reviewersbz
bugs1405087
milestone60.0a1
Bug 1405087: Don't use -moz-user-input: disabled to decide event handling stuff. r?bz We only set it on disabled form controls anyway, so use the content state directly. MozReview-Commit-ID: 7jJ75dvszyC
dom/html/HTMLOptGroupElement.cpp
dom/html/nsGenericHTMLElement.cpp
layout/base/nsCaret.cpp
layout/forms/nsCheckboxRadioFrame.cpp
layout/forms/nsComboboxControlFrame.cpp
layout/forms/nsGfxButtonControlFrame.cpp
layout/forms/nsImageControlFrame.cpp
layout/forms/nsListControlFrame.cpp
layout/generic/nsFrame.cpp
layout/generic/nsIFrame.h
--- a/dom/html/HTMLOptGroupElement.cpp
+++ b/dom/html/HTMLOptGroupElement.cpp
@@ -41,25 +41,24 @@ NS_IMPL_ELEMENT_CLONE(HTMLOptGroupElemen
 
 
 nsresult
 HTMLOptGroupElement::GetEventTargetParent(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;
     }
   }
 
   return nsGenericHTMLElement::GetEventTargetParent(aVisitor);
 }
 
 Element*
--- a/dom/html/nsGenericHTMLElement.cpp
+++ b/dom/html/nsGenericHTMLElement.cpp
@@ -2276,24 +2276,24 @@ nsGenericHTMLFormElement::IsElementDisab
     case eWheel:
     case eLegacyMouseLineOrPageScroll:
     case eLegacyMousePixelScroll:
       return false;
     default:
       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
 nsGenericHTMLFormElement::UpdateFormOwner(bool aBindToTree,
                                           Element* aFormIdElement)
 {
   NS_PRECONDITION(!aBindToTree || !aFormIdElement,
                   "aFormIdElement shouldn't be set if aBindToTree is true!");
--- a/layout/base/nsCaret.cpp
+++ b/layout/base/nsCaret.cpp
@@ -512,28 +512,27 @@ nsCaret::GetPaintGeometry(nsRect* aRect)
     return nullptr;
   }
 
   // Update selection language direction now so the new direction will be
   // taken into account when computing the caret position below.
   CheckSelectionLanguageChange();
 
   int32_t frameOffset;
-  nsIFrame *frame = GetFrameAndOffset(GetSelectionInternal(),
+  nsIFrame* frame = GetFrameAndOffset(GetSelectionInternal(),
       mOverrideContent, mOverrideOffset, &frameOffset);
   if (!frame) {
     return nullptr;
   }
 
   // now we have a frame, check whether it's appropriate to show the caret here
   const nsStyleUserInterface* userinterface = frame->StyleUserInterface();
   if ((!mIgnoreUserModify &&
        userinterface->mUserModify == StyleUserModify::ReadOnly) ||
-      userinterface->mUserInput == StyleUserInput::None ||
-      userinterface->mUserInput == StyleUserInput::Disabled) {
+      frame->IsContentDisabled()) {
     return nullptr;
   }
 
   // If the offset falls outside of the frame, then don't paint the caret.
   int32_t startOffset, endOffset;
   if (frame->IsTextFrame() &&
       (NS_FAILED(frame->GetOffsets(startOffset, endOffset)) ||
        startOffset > frameOffset || endOffset < frameOffset)) {
--- a/layout/forms/nsCheckboxRadioFrame.cpp
+++ b/layout/forms/nsCheckboxRadioFrame.cpp
@@ -180,20 +180,18 @@ nsCheckboxRadioFrame::SetFocus(bool aOn,
 {
 }
 
 nsresult
 nsCheckboxRadioFrame::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;
 }
 
 void
 nsCheckboxRadioFrame::GetCurrentCheckState(bool* aState)
 {
--- a/layout/forms/nsComboboxControlFrame.cpp
+++ b/layout/forms/nsComboboxControlFrame.cpp
@@ -1184,19 +1184,17 @@ nsComboboxControlFrame::HandleEvent(nsPr
       *aEventStatus = nsEventStatus_eConsumeNoDefault;
       return NS_OK;
     }
   }
 #endif
 
   // 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;
 }
 
 
 nsresult
 nsComboboxControlFrame::SetFormProperty(nsAtom* aName, const nsAString& aValue)
--- a/layout/forms/nsGfxButtonControlFrame.cpp
+++ b/layout/forms/nsGfxButtonControlFrame.cpp
@@ -185,16 +185,13 @@ nsresult
 nsGfxButtonControlFrame::HandleEvent(nsPresContext* aPresContext,
                                      WidgetGUIEvent* aEvent,
                                      nsEventStatus* aEventStatus)
 {
   // Override the HandleEvent to prevent the nsFrame::HandleEvent
   // 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;
 }
--- a/layout/forms/nsImageControlFrame.cpp
+++ b/layout/forms/nsImageControlFrame.cpp
@@ -138,28 +138,20 @@ nsImageControlFrame::HandleEvent(nsPresC
 {
   NS_ENSURE_ARG_POINTER(aEventStatus);
 
   // Don't do anything if the event has already been handled by someone
   if (nsEventStatus_eConsumeNoDefault == *aEventStatus) {
     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->AsElement()->HasAttr(kNameSpaceID_None, nsGkAtoms::disabled)) {
-    // XXX cache disabled
-    return NS_OK;
-  }
-
   *aEventStatus = nsEventStatus_eIgnore;
 
   if (aEvent->mMessage == eMouseUp &&
       aEvent->AsMouseEvent()->button == WidgetMouseEvent::eLeftButton) {
     // Store click point for HTMLInputElement::SubmitNamesValues
     // Do this on MouseUp because the specs don't say and that's what IE does
     nsIntPoint* lastClickPoint =
       static_cast<nsIntPoint*>
--- a/layout/forms/nsListControlFrame.cpp
+++ b/layout/forms/nsListControlFrame.cpp
@@ -920,26 +920,21 @@ nsListControlFrame::HandleEvent(nsPresCo
     printf("Mouse in ListFrame %s [%d]\n", desc[inx], aEvent->mMessage);
   } else {
     printf("Mouse in ListFrame <UNKNOWN> [%d]\n", aEvent->mMessage);
   }*/
 
   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);
 }
 
 
 //---------------------------------------------------------
 void
 nsListControlFrame::SetInitialChildList(ChildListID    aListID,
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -6542,16 +6542,29 @@ nsFrame::Reflow(nsPresContext*          
 {
   MarkInReflow();
   DO_GLOBAL_REFLOW_COUNT("nsFrame");
   MOZ_ASSERT(aStatus.IsEmpty(), "Caller should pass a fresh reflow status!");
   aDesiredSize.ClearSize();
   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)
 {
   NS_NOTREACHED("should only be called for text frames");
   return NS_OK;
 }
 
 nsresult
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -2781,16 +2781,21 @@ public:
   /**
    * Same as GetNearestWidget() above but uses an outparam to return the offset
    * of this frame to the returned widget expressed in appunits of |this| (the
    * widget might be in a different document with a different zoom).
    */
   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.
    *
    * @see mozilla::LayoutFrameType
    */
   mozilla::LayoutFrameType Type() const {
     MOZ_ASSERT(uint8_t(mClass) < mozilla::ArrayLength(sLayoutFrameTypes));
     return sLayoutFrameTypes[uint8_t(mClass)];
   }