487023 - Refactor IsCaseSensitive() into non-virtual inline IsHTML()

Status: RESOLVED FIXED

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

Description

[Henri Sivonen (:hsivonen)]

nsIDocument has virtual PRBool IsCaseSensitive(). The common usage patterns for this method really is that !IsCaseSensitive() means that the document is an HTML document for the purposes of HTML5. Therefore, it would be more intuitive to have a method IsHTML() that returned the negation of what IsCaseSensitive() now returns.

Moreover, it seems useless that this method is virtual. I seems to me that it could be marginally better for perf to make it a non-virtual inline and to hoist mIsRegularHTML from nsHTMLDocument to nsIDocument.

Also, IsXHTML() on nsHTMLDocument is redundant with IsCaseSensitive() and should probably be refactored into !IsHTML().

Marking as depending on bug 468708, because merge conflicts with landing that one would not be nice.

Comment 1

[David Zbarsky (:dzbarsky)]

Attachment: Patch

Comment 2

[Henri Sivonen (:hsivonen)]

Comment on attachment 391265 [details] [diff] [review]
Patch

>  // XXXbz should mIsRegularHTML be reset if someone manually calls
> // SetContentType() on this document?

Per HTML5, the only way to change the HTMLness flag is to call document.open() on a Document object.

> if (IsHTML())
>   return nsStyleSet::eHTMLPresHintSheet;

I'd use curly braces around the second line even though it is a single-statement block.

> if (IsHTML())
>   rv = GetElementsByTagName(NS_LITERAL_STRING("frameset"),
>  getter_AddRefs(nodeList));
> else

Curly braces would be good here, too.

With those curly braces, r=hsivonen. Thanks for fixing this!

Comment 3

[Henri Sivonen (:hsivonen)]

Comment on attachment 391265 [details] [diff] [review]
Patch

Actually, I'm not sure if I'm authorized to grant an r+ on this code. I'll check.

Comment 4

[Henri Sivonen (:hsivonen)]

Comment on attachment 391265 [details] [diff] [review]
Patch

I checked. You should still get an sr from a content module peer before pushing to m-c.

Comment 5

[David Zbarsky (:dzbarsky)]

Attachment: Patch for check-in

Comment 6

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/2f27256aa154