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

VERIFIED FIXED in mozilla1.6alpha

Status

()

Core
Layout
P1
normal
VERIFIED FIXED
14 years ago
14 years ago

People

(Reporter: RafG, Assigned: bz)

Tracking

({regression, testcase, xhtml})

Trunk
mozilla1.6alpha
x86
All
regression, testcase, xhtml
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

14 years ago
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.

Comment 1

14 years ago
Confirm 20030729 PC/WinXP.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

14 years ago
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)?
Created attachment 132941 [details] [diff] [review]
Patch
Created attachment 132942 [details] [diff] [review]
Same as diff -w, FOR REVIEW ONLY
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
Last Resolved: 14 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

Updated

14 years ago
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.