Convert HTMLLegendElement to WebIDL

RESOLVED FIXED in mozilla21

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: baku, Assigned: baku)

Tracking

(Blocks: 1 bug)

Trunk
mozilla21
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 7 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

5 years ago
Created attachment 710640 [details] [diff] [review]
patch 1 - renaming nsHTMLLegendElement to HTMLLegendElement
Attachment #710640 - Flags: review?(peterv)
(Assignee)

Comment 2

5 years ago
Created attachment 710641 [details] [diff] [review]
patch - webidl
Attachment #710641 - Flags: review?(peterv)
(Assignee)

Comment 4

5 years ago
Created attachment 711253 [details] [diff] [review]
part 1 - renaming

small indentation issue
Attachment #710640 - Attachment is obsolete: true
Attachment #710640 - Flags: review?(peterv)
Attachment #711253 - Flags: review?(Ms2ger)
(Assignee)

Comment 5

5 years ago
Created attachment 711254 [details] [diff] [review]
part 2 - webidl
Attachment #710641 - Attachment is obsolete: true
Attachment #710641 - Flags: review?(peterv)
Attachment #711254 - Flags: review?(Ms2ger)
Comment on attachment 711253 [details] [diff] [review]
part 1 - renaming

Review of attachment 711253 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/nsHTMLLegendElement.cpp
@@ +2,5 @@
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#include "HTMLLegendElement.h"

mozilla/dom/. Also above and below.

::: content/html/content/src/nsHTMLLegendElement.h
@@ +2,5 @@
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +#ifndef HTMLLegendElement_h___
> +#define HTMLLegendElement_h___

mozilla_dom_HTMLLegendElement_h

@@ +64,5 @@
>                               bool aNotify);
>  
>    virtual nsresult Clone(nsINodeInfo *aNodeInfo, nsINode **aResult) const;
>  
> +  Element *GetFormElement()

* to the left while you're here

::: dom/base/nsDOMClassInfo.cpp
@@ +6984,5 @@
>        else {
>          // Legend isn't an HTML form control but should have its fieldset form
>          // as scope parent at least for backward compatibility.
> +        HTMLLegendElement *legend =
> +          HTMLLegendElement::FromContent(node->AsElement());

I want someone else to make sure nothing needs to change here.
Attachment #711253 - Flags: review?(peterv)
Attachment #711253 - Flags: review?(Ms2ger)
Attachment #711253 - Flags: review+
Comment on attachment 711254 [details] [diff] [review]
part 2 - webidl

Review of attachment 711254 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/HTMLLegendElement.cpp
@@ +173,5 @@
>  
> +already_AddRefed<nsHTMLFormElement>
> +HTMLLegendElement::GetForm()
> +{
> +  Element* form = GetFormElement();

This needs a MOZ_ASSERT_IF(form, form->IsHTML(nsGkAtoms::form));

::: content/html/content/src/HTMLLegendElement.h
@@ +6,5 @@
>  #define HTMLLegendElement_h___
>  
>  #include "nsIDOMHTMLLegendElement.h"
>  #include "nsGenericHTMLElement.h"
> +#include "nsHTMLFormElement.h"

A forward declaration suffices.

::: dom/webidl/HTMLLegendElement.webidl
@@ +16,5 @@
> +interface HTMLLegendElement : HTMLElement {
> +  readonly attribute HTMLFormElement? form;
> +};
> +
> +// http://www.whatwg.org/specs/web-apps/current-work/#dom-hr-color

Same nit as before
Attachment #711254 - Flags: review?(Ms2ger) → review+
(Assignee)

Comment 8

5 years ago
Created attachment 711390 [details] [diff] [review]
part 2 - webidl

https://tbpl.mozilla.org/?tree=Try&rev=0a1e9cb05950 green on try
Attachment #711254 - Attachment is obsolete: true
Attachment #711390 - Flags: review+

Updated

5 years ago
Attachment #711253 - Flags: review?(peterv)
Comment on attachment 711390 [details] [diff] [review]
part 2 - webidl

Review of attachment 711390 [details] [diff] [review]:
-----------------------------------------------------------------

I talked to bz; you need a GetParentObject() that returns GetFormElement(), if it's not null.
Attachment #711390 - Flags: review+ → review-
(Assignee)

Comment 10

5 years ago
Created attachment 714046 [details] [diff] [review]
part 2 - webidl
Attachment #711390 - Attachment is obsolete: true
Attachment #714046 - Flags: review?(Ms2ger)
Comment on attachment 714046 [details] [diff] [review]
part 2 - webidl

Review of attachment 714046 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/HTMLLegendElement.h
@@ +92,5 @@
> +
> +  nsWrapperCache* GetParentObject() {
> +    Element* form = GetFormElement();
> +    return form ? static_cast<nsWrapperCache*>(form)
> +                : nsGenericHTMLElement::GetParentObject();

I would return an nsINode, fwiw.
Attachment #714046 - Flags: review?(Ms2ger) → review+
(Assignee)

Comment 12

5 years ago
Created attachment 714320 [details] [diff] [review]
part 2 - webidl

https://tbpl.mozilla.org/?tree=Try&rev=0cc3662213b5
Attachment #714046 - Attachment is obsolete: true
Attachment #714320 - Flags: review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
(Assignee)

Comment 13

5 years ago
Created attachment 714417 [details] [diff] [review]
part 1 - renaming
Attachment #711253 - Attachment is obsolete: true
Attachment #714417 - Flags: review+
(Assignee)

Comment 14

5 years ago
Created attachment 714418 [details] [diff] [review]
part 2 - webidl
Attachment #714320 - Attachment is obsolete: true
Attachment #714418 - Flags: review+
Depends on: 844136
You need to log in before you can comment on or make changes to this bug.