Closed Bug 1403795 Opened 2 years ago Closed 2 years ago

Remove nsIDOMHTMLButtonElement

Categories

(Core :: DOM: Core & HTML, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: qdot, Assigned: qdot)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Continuing post-addon-deprecation XPCOM interface cleanup
Attachment #8913027 - Flags: review?(bzbarsky)
Comment on attachment 8913027 [details]
Bug 1403795 - Remove nsIDOMHTMLButtonElement;

https://reviewboard.mozilla.org/r/184388/#review189992

r=me

::: dom/html/HTMLButtonElement.h:118
(Diff revision 1)
>    {
>      SetHTMLBoolAttr(nsGkAtoms::disabled, aDisabled, aError);
>    }
>    // nsGenericHTMLFormElement::GetForm is fine.
>    using nsGenericHTMLFormElement::GetForm;
> -  // XPCOM GetFormAction is fine.
> +  void GetFormAction(nsAString& aFormAction)

Actually, this GetFormAction should just go away; it's more or less dead code.  That "XPCOM GetFormAction is fine" comment is a lie; that hasn't been true since bug 1366361 was fixed.  It should really say "GetFormAction implemented in superclass" or so.

You should also be able to drop the `using nsGenericHTMLFormElementWithState::GetFormAction` bit, now that you won't be overriding it in this class.

::: dom/html/HTMLButtonElement.cpp:463
(Diff revision 1)
>  
>  bool
>  HTMLButtonElement::RestoreState(nsPresState* aState)
>  {
>    if (aState && aState->IsDisabledSet() && !aState->GetDisabled()) {
> -    SetDisabled(false);
> +    ErrorResult rv;

IgnoredErrorResult, since you do ignore it...

::: dom/html/HTMLFormElement.cpp:1652
(Diff revision 1)
>  
>      nsCOMPtr<nsIDOMHTMLInputElement> inputElement = do_QueryInterface(aOriginatingElement);
>      if (inputElement) {
>        inputElement->GetFormAction(action);
>      } else {
> -      nsCOMPtr<nsIDOMHTMLButtonElement> buttonElement = do_QueryInterface(aOriginatingElement);
> +      RefPtr<HTMLButtonElement> buttonElement = HTMLButtonElement::FromContent(aOriginatingElement);

You could probably just do:

    auto buttonElement = HTMLButtonElement::FromContent(aOriginatingElement);
  
because I don't think you need a strong ref here.
Attachment #8913027 - Flags: review?(bzbarsky) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s ccf0a9399c9a -d 9f78e0d0d331: rebasing 423440:ccf0a9399c9a "Bug 1403795 - Remove nsIDOMHTMLButtonElement; r=bz" (tip)
merging dom/interfaces/html/moz.build
merging mobile/android/modules/FormAssistant.jsm
merging xpcom/reflect/xptinfo/ShimInterfaceInfo.cpp
warning: conflicts while merging mobile/android/modules/FormAssistant.jsm! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
https://hg.mozilla.org/mozilla-central/rev/03d936e8aec2
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.