Last Comment Bug 214969 - using application/xhtml+xml, the background of body fills entire viewport
: using application/xhtml+xml, the background of body fills entire viewport
Status: VERIFIED FIXED
: regression, testcase, xhtml
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: x86 All
: P1 normal (vote)
: mozilla1.6alpha
Assigned To: Boris Zbarsky [:bz]
: Hixie (not reading bugmail)
Mentors:
http://users.pandora.be/rguns/test.xhtml
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2003-08-03 16:28 PDT by RafG
Modified: 2004-03-01 10:05 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (5.15 KB, patch)
2003-10-09 11:32 PDT, Boris Zbarsky [:bz]
no flags Details | Diff | Review
Same as diff -w, FOR REVIEW ONLY (4.02 KB, patch)
2003-10-09 11:33 PDT, Boris Zbarsky [:bz]
dbaron: review+
dbaron: superreview+
Details | Diff | Review

Description RafG 2003-08-03 16:28:34 PDT
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 Bill Mason 2003-08-03 17:05:56 PDT
Confirm 20030729 PC/WinXP.
Comment 2 Andrew Schultz 2003-08-03 20:34:43 PDT
behavior changed between linux trunk 2003041805 and 2003041905, probably bug 111514
Comment 3 Boris Zbarsky [:bz] 2003-08-09 17:41:13 PDT
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)?
Comment 4 Boris Zbarsky [:bz] 2003-10-09 11:32:14 PDT
Created attachment 132941 [details] [diff] [review]
Patch
Comment 5 Boris Zbarsky [:bz] 2003-10-09 11:33:02 PDT
Created attachment 132942 [details] [diff] [review]
Same as diff -w, FOR REVIEW ONLY
Comment 6 Boris Zbarsky [:bz] 2003-10-09 11:35:15 PDT
Taking
Comment 7 Boris Zbarsky [:bz] 2003-10-09 11:41:48 PDT
Another option, which jst is ok with is to add an IsContentOfType(eXHTML) so we
could do:

  IsContentOfType(eHTML) && !IsContentOfType(eXHTML)

Comment 8 Hixie (not reading bugmail) 2003-10-09 11:49:02 PDT
So IsContentOfType(eXHTML) would imply IsContentOfType(eHTML) but not vice 
versa?
Comment 9 Boris Zbarsky [:bz] 2003-10-09 12:13:31 PDT
That's probably what we would do if we added that, yes.
Comment 10 Christopher Aillon (sabbatical, not receiving bugmail) 2003-10-14 14:19:30 PDT
Re: comment 3, we already expose the IsXHTML() thing.
See nsIDocument::IsCaseSensitive()
Comment 11 Boris Zbarsky [:bz] 2003-10-14 20:56:43 PDT
I think using that to mean "is XHTML?" would be somewhat obscure... So I've gone
ahead with the patch as-is.

Fixed.
Comment 12 Boris Zbarsky [:bz] 2003-10-14 22:56:01 PDT
I had to change to caillon's suggestion after all, because HTML elements lie
about their namespace ID... :(
Comment 13 Hixie (not reading bugmail) 2003-10-15 05:38:29 PDT
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...).
Comment 14 Boris Zbarsky [:bz] 2003-10-15 09:26:03 PDT
> 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.
Comment 15 Hixie (not reading bugmail) 2003-10-15 13:55:10 PDT
fair enough

Note You need to log in before you can comment on or make changes to this bug.