Open Bug 222194 Opened 21 years ago Updated 2 years ago

XUL boxes don't wrap inline children in anonymous block

Categories

(Core :: XUL, defect)

x86
Windows 98
defect

Tracking

()

People

(Reporter: WeirdAl, Unassigned)

References

()

Details

(Keywords: testcase)

Attachments

(2 obsolete files)

Currently, MathML inside an XUL document renders very strangely.  The URL in
this bug leads to a testcase I crafted showing XUL > MathML and XUL > XHTML >
MathML testcases for Math.pow(2, (x + 2)).

To render MathML in XUL at all, the MathML needs to be either inside a block or
styled as a block itself.  Patch coming up for that in a few minutes.

Also, on Windows 98SE, the plus sign appears to have a second level of
superscript applying to it if you look closely.  bz reports correct rendering
via RedHat Linux 8.0; I'll check it in Mandrake Linux 9.1 in a few minutes.
http://weblogs.mozillazine.org/weirdal/archives/MathML_test_win98SE.png

Screenshot under Windows with math element as a block.
It is not really MathML specific. The same (empty) result happens by replacing 
<math xmlns="http://www.w3.org/1998/Math/MathML"> 
  ... 
</math> 

with 

<span xmlns="http://www.w3.org/1999/xhtml">
  Hello World
</span>

The cause of the oddity is an early return in nsInlineFrame::Reflow() :

  if (nsnull == aReflowState.mLineLayout) {
    return NS_ERROR_INVALID_ARG;
  }

i.e., XUL is asking to reflow inline frames without providing a linelayout object.

Re-assigning to XUL. If the misaligment of the '+' is still a problem, do well
to report it as a separate bug. On my Win2K box, it looks fine.
Assignee: rbs → hyatt
Component: MathML → XP Toolkit/Widgets: XUL
QA Contact: ian → shrir
Summary: XUL > MathML rendering oddities → XUL > inline elements have rendering oddities
All of a XUL box's children should become blocks.  This was never implemented, but it should be 
done at some point.  If an inline child is ever encountered inside a XUL box, it should be wrapped 
in an anonymous block.  Right now this doesn't happen, but it should.  Similarly, nsIFrames 
shouldn't be made for whitespace encountered inside XUL boxes.  Basically all the same rules as a 
block with block-only children apply.
This is how the XUL box model in Safari works.
Comment on attachment 133316 [details] [diff] [review]
patch to xul.css for * > math|math { display:block; }

hyatt: I know xul.css is locked down.  I tried modifying
resource:///res/mathml.css in the source tree to achieve the same result; no
effect.

If this change is not acceptable, please advise on what I should do.
Attachment #133316 - Flags: review?(hyatt)
I'm reluctant to pollute xul.css with mathml style rules.  MathML's user agent CSS file should 
contain the fix IMO.
I tried fixing mathml.css.  I just couldn't make it work.  (I agree, though,
that a MathML-specific rule is not a very good solution.  It was just the first
I could make work.)

What about a more generic rule, one that selects children which aren't in the
XUL namespace?
Well, there is actually a C++ code problem with box in that it doesn't wrap inlines in anonymous 
blocks.  It would be nice to fix that problem.
Attached file patch, v2 (obsolete) —
hyatt: well, if you're willing to accept a CSS-based solution to the problem,
here's one.  (I am listening to you, but unfortunately, I was already working
out this solution when you posted your most recent comment.)

It adds a new file, chrome://global/content/xul_negate.css, which properly
performs the selector and restores a default of display:block.
Attachment #133316 - Attachment is obsolete: true
That wouldn't work. The XUL box is set by the 'display' property, not by virtue 
of elements being in the XUL namespace.

You could totally replicate this bug using only elements from the HTML 
namespace.
QA Contact: shrir → ian
Summary: XUL > inline elements have rendering oddities → XUL boxes don't wrap inline children in anonymous block
Comment on attachment 133318 [details]
patch, v2

Or any XML language.

<foo:bar><foo:baz/></foo:bar>

foo|bar {
  display: -moz-box;
}

Well, even Barry Bonds strikes out once in a while... and I'm more like the
batboy.
Attachment #133318 - Attachment is patch: false
> I tried modifying
> resource:///res/mathml.css in the source tree to achieve the same result; no
> effect.

I noted that mathml.css is not automatically loaded in XUL documents that have
MathML elements. For performance reasons (tm), there is some code to load
mathml.css on demand -- if/when MathML elements are encountered on a page. This
 way, the MathML rules don't come into play in the majority of documents that
don't have MathML markup. The code that does that is currently in
nsXMLContentSink.cpp :

MathMLElementFactoryImpl::CreateInstanceByTag(nsINodeInfo* aNodeInfo,
                                              nsIContent** aResult)
{
  static const char kMathMLStyleSheetURI[] = "resource://gre/res/mathml.css";

  // this bit of code is to load mathml.css on demand
  nsIDocument* doc = aNodeInfo->GetDocument();
  if (doc) {
    [... load mathml.css ...]
  }
}

It appears that the doc is null when called from XUL. This even causes MathML in
XUL not to be styled properly.

You might want to include "resource://gre/res/mathml.css" in your XUL, and your
modifications should also take effect.
hyatt: I'm reading through the source code on LXR to get a feel for a starting
location, and I think I've found it.

-moz-box leads directly to _moz_box as nsCSSKeywordList.h#78
That leads to NS_STYLE_DISPLAY_BOX in nsCSSProps.cpp#442
That, in turn, leads to nsCSSFrameConstructor.cpp#5464.

There, a couple of variables within the ConstructXULFrame method are set to true
instead of false.  (Line 5187 for the original variable values.)  The variables
are processChildren and isReplaced.  Unfortunately, other conditional statements
which don't rely on NS_STYLE_DISPLAY_BOX also set those two variables...

This may be a wild goose chase, but it seems a logical starting point.  Do we
need a new method specifically for NS_STYLE_DISPLAY_BOX within
nsCSSFrameConstructor?  (I ask because the same if statement using
NS_STYLE_DISPLAY_BOX also lets us into its statement block with
NS_STYLE_DISPLAY_INLINE_BOX.)
Attachment #133318 - Attachment is obsolete: true
Attachment #133316 - Flags: review?(hyatt)
Alex:
Is it possible that SVG suffers from the same problems ?
So the place to do this would either be at
http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsBoxToBlockAdaptor.cpp#153
(replacing the DEBUG_waterson stuff with code that creates an anonymous block
and all that) or at
http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsContainerBox.cpp#127
(making sure we wrap before creating an adaptor).

Which behavior is chosen would probably depend on whether we wanted multiple
inlines/tables/whatever in a row to all end up in one box or multiple boxes.

There's also the issue of how the frame lists should be linked if this approach
is taken.... if we just leave the inlines linked as siblings, will reflow get
confused?
Hyatt? What was it we decided when it comes to wrapping inlines inside boxes?
re comment 16, Boris, why shouldn't the frame be created inside the nsCSSFrameConstructor.cpp
Blocks: 317682
It could be, I guess.... I suppose if we make the table pseudo-frame thing work better we could try to reuse that infrastructure.
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: ian → xptoolkit.widgets
Assignee: hyatt → nobody
I believe dbaron fixed this a while back.
Yes, although the fix wasn't ideal since we wrap all the kids of the XUL box in an anonymous box if some of them are inlines, rather than wrapping the contiguous inline segments.  That was done in bug 321402 and bug 421203.
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: