Closed Bug 827149 Opened 11 years ago Closed 11 years ago

Remove some uses of nsIDOMHTMLBodyElement

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: dzbarsky, Assigned: dzbarsky)

Details

Attachments

(1 file)

      No description provided.
Attached patch PatchSplinter Review
Attachment #698513 - Flags: review?(bzbarsky)
Comment on attachment 698513 [details] [diff] [review]
Patch

So this adds an extra function call to GetBodyElement.  That's somewhat unfortunate...

It's tempting to add an nsIDocumentInlines header where stuff like a GetBodyElement that includes HTMLBodyElement.h could go.  Thoughts?

> nsGenericHTMLElement::IsCurrentBodyElement()
>+  if (!IsHTML(nsGkAtoms::body)) {

We know we're HTML.  Just compare Tag() to body?

>+  return htmlElement == static_cast<HTMLBodyElement*>(this);

Really, what would be best is if we got our hands on an nsHTMLDocument and returned GetBody() == this.  File a followup on a simple way to get an nsHTMLDocument* from nsIDocument* if it's HTML, perhaps?

The various link color bits are actually wrong per spec.  If the <html> has a child frameset and then a child body they'll do the wrong thing.  You're not adding that breakage, but it means that this code can't really use GetBodyElement long-term.

In fact, it could be argued that call GetBodyElement consumers are broken in that situation...

Note that our GetBody() is similarly broken.  Perhaps the spec should be changed, depending on what other UAs do.

r=me modulo all that
Attachment #698513 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky (:bz) from comment #2)
> Comment on attachment 698513 [details] [diff] [review]
> Patch
> 
> So this adds an extra function call to GetBodyElement.  That's somewhat
> unfortunate...
> 
> It's tempting to add an nsIDocumentInlines header where stuff like a
> GetBodyElement that includes HTMLBodyElement.h could go.  Thoughts?
> 

Makes sense.  I added that.

> 
> The various link color bits are actually wrong per spec.  If the <html> has
> a child frameset and then a child body they'll do the wrong thing.  You're
> not adding that breakage, but it means that this code can't really use
> GetBodyElement long-term.
> 
> In fact, it could be argued that call GetBodyElement consumers are broken in
> that situation...
> 
> Note that our GetBody() is similarly broken.  Perhaps the spec should be
> changed, depending on what other UAs do.

Followup ok?
> Followup ok?

Absolutely!
https://hg.mozilla.org/mozilla-central/rev/7293053de117
https://hg.mozilla.org/mozilla-central/rev/ee10b02f78df
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.