Closed Bug 214969 Opened 22 years ago Closed 21 years ago

using application/xhtml+xml, the background of body fills entire viewport

Categories

(Core :: Layout, defect, P1)

Product:
Core
Component:
Layout
Platform:
x86
All
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.6alpha

People

(Reporter: raf+bugzilla1, Assigned: bzbarsky)

References

(
URL
)

Details

(Keywords: regression, testcase, xhtml)

Attachments

(2 files)

Patch
5.15 KB, patch
Details | Diff | Splinter Review
Same as diff -w, FOR REVIEW ONLY
4.02 KB, patch
dbaron
: review+
dbaron
: superreview+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.4) Gecko/20030624 Build Identifier: Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.4) Gecko/20030624 When visiting an XHTML page with MIME type application/xhtml+xml, Mozilla 1.4 fills the entire viewport with the background-color of the body element. This is against the specs, see e.g.: http://www.w3.org/TR/CSS21/colors.html#q2 http://bugzilla.mozilla.org/show_bug.cgi?id=147856#c2 Reproducible: Always Steps to Reproduce: 1. Visit demonstration URL with Mozilla 1.4 or up. Actual Results: The body's background-color is applied to the entire viewport. Expected Results: The body's background-color should not extend beyond the body's content. This seems to be a regression that happened somewhere inbetween Moz 1.3.1 and Moz 1.4, -- Mozilla 1.3.1 still performs correctly. I can reproduce it on both Windows and Linux.
Confirm 20030729 PC/WinXP.
Status: UNCONFIRMED → NEW
Ever confirmed: true
behavior changed between linux trunk 2003041805 and 2003041905, probably bug 111514
Keywords: regression, testcase
Indeed, the relevant code is in FindCanvasBackground() in nsCSSRendering.cpp and this just QIs the document to nsIDOMHTMLDocument... We could expose the IsXHTML() thing on nsIHTMLDocument maybe? Or check that the namespace of the body nsIContent node is kNameSpaceID_None (which means HTML, not XHTML)?
Attached patch Patch — — Splinter Review
Attachment #132942 - Flags: superreview?(dbaron)
Attachment #132942 - Flags: review?(dbaron)
Taking
Assignee: other → bzbarsky
Priority: -- → P1
Summary: using application/xhtml+xml, the background of body fills entire viewport → [FIX]using application/xhtml+xml, the background of body fills entire viewport
Target Milestone: --- → mozilla1.6alpha
Another option, which jst is ok with is to add an IsContentOfType(eXHTML) so we could do: IsContentOfType(eHTML) && !IsContentOfType(eXHTML)
So IsContentOfType(eXHTML) would imply IsContentOfType(eHTML) but not vice versa?
That's probably what we would do if we added that, yes.
Attachment #132942 - Flags: superreview?(dbaron)
Attachment #132942 - Flags: superreview+
Attachment #132942 - Flags: review?(dbaron)
Attachment #132942 - Flags: review+
Re: comment 3, we already expose the IsXHTML() thing. See nsIDocument::IsCaseSensitive()
I think using that to mean "is XHTML?" would be somewhat obscure... So I've gone ahead with the patch as-is. Fixed.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
I had to change to caillon's suggestion after all, because HTML elements lie about their namespace ID... :(
To be honest using the case sensitivity thing instead of namespaces is probably better anyway, since anyone can send any XML as application/xhtml+xml, resulting in an HTML DOM, even if the document isn't HTML, for example: <html> <body> </body> </html> ...and in that case we wouldn't want to propagate the background. Any chance we can make IsCaseSensitive() slightly more obvious about what it really means? Or, maybe better, have IsHTML() and IsCaseSensitive() be separate methods, since if we ever implement a case-insensitive other language, we don't want to do this propagation stuff for it (not to mention it makes the code a lot easier to read if it actually looks like it's checking what it really is checking...).
> for example Nope. That markup, in a document sent as application/xhtml+xml, would not lead to HTMLElement objects being created. > since if we ever implement a case-insensitive other language At the moment we are checking two things: 1) This is an HTML document (QIs to nsIDOMHTMLDocument) 2) It's case-insensitive (so not XHTML) Our implementing another language would change nothing here unless all documents started QIing to nsIDOMHTMLDocument; in that case we would need more accessors on nsIDocument in any case.
fair enough
Status: RESOLVED → VERIFIED
Keywords: xhtml
Summary: [FIX]using application/xhtml+xml, the background of body fills entire viewport → using application/xhtml+xml, the background of body fills entire viewport
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: