Commit f80234e7 authored by Mirko Brodesser's avatar Mirko Brodesser
Browse files

Bug 1831081: when the popover attribute changed, update the popover attribute... 

Bug 1831081: when the popover attribute changed, update the popover attribute state before hiding the popover. r=emilio

Moreover, simplify the code to match the spec more closely.

Differential Revision: https://phabricator.services.mozilla.com/D178906
parent cdc932a3

dom/html/nsGenericHTMLElement.cpp

+34 −23
Original line number Diff line number Diff line
@@ -688,32 +688,41 @@ constexpr PopoverAttributeState ToPopoverAttributeState( 
void nsGenericHTMLElement::AfterSetPopoverAttr() {

  const nsAttrValue* newValue = GetParsedAttr(nsGkAtoms::popover);



  PopoverAttributeState newState;

  const PopoverAttributeState newState = [&newValue]() {

    if (newValue) {

      MOZ_ASSERT(newValue->Type() == nsAttrValue::eEnum);

    const PopoverAttributeKeyword popoverAttributeKeyword =

      const auto popoverAttributeKeyword =

          static_cast<PopoverAttributeKeyword>(newValue->GetEnumValue());

    newState = ToPopoverAttributeState(popoverAttributeKeyword);

  } else {

      return ToPopoverAttributeState(popoverAttributeKeyword);

    }



    // The missing value default is the no popover state, see

    // <https://html.spec.whatwg.org/multipage/popover.html#attr-popover>.

    newState = PopoverAttributeState::None;

  }

    return PopoverAttributeState::None;

  }();



  const PopoverAttributeState oldState = GetPopoverAttributeState();



  PopoverAttributeState oldState = GetPopoverAttributeState();

  if (newState != oldState) {

    if (oldState != PopoverAttributeState::None) {

    if (newState == PopoverAttributeState::None) {

      ClearPopoverData();

    } else {

      EnsurePopoverData().SetPopoverAttributeState(newState);

    }



    HidePopoverInternal(/* aFocusPreviousElement = */ true,

                        /* aFireEvents = */ true, IgnoreErrors());

    }

    // Bug 1831081: `newState` might here differ from

    // `GetPopoverAttributeState()`.

    if (newState != PopoverAttributeState::None) {

      EnsurePopoverData().SetPopoverAttributeState(newState);

      PopoverPseudoStateUpdate(false, true);

    } else {



    // In case `HidePopoverInternal` changed the state, keep the corresponding

    // changes and don't overwrite anything here.

    if (newState == GetPopoverAttributeState()) {

      if (newState == PopoverAttributeState::None) {

        ClearPopoverData();

        RemoveStates(ElementState::POPOVER_OPEN);

      } else {

        // TODO: what if `HidePopoverInternal` called `ShowPopup()`?

        PopoverPseudoStateUpdate(false, true);

      }

    }

  }

}
@@ -3226,12 +3235,14 @@ bool nsGenericHTMLElement::CheckPopoverValidity( 
    ErrorResult& aRv) {

  const PopoverData* data = GetPopoverData();

  if (!data ||

      data->GetPopoverAttributeState() == PopoverAttributeState::None ||

      !HasAttr(nsGkAtoms::popover) /* TODO: revisit this in bug 1831081. */) {

      data->GetPopoverAttributeState() == PopoverAttributeState::None) {

    MOZ_ASSERT(!HasAttr(nsGkAtoms::popover));

    aRv.ThrowNotSupportedError("Element is in the no popover state");

    return false;

  }



  MOZ_ASSERT(HasAttr(nsGkAtoms::popover));



  if (data->GetPopoverVisibilityState() != aExpectedState) {

    return false;

  }