Closed Bug 9458 (inline-block) Opened 25 years ago Closed 17 years ago

Implement inline-block in layout

Categories

(Core :: Layout: Block and Inline, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9alpha8

People

(Reporter: sujay, Assigned: dbaron)

References

(Blocks 2 open bugs, )

Details

(4 keywords, Whiteboard: [A2][patch])

Attachments

(9 files, 6 obsolete files)

1.42 KB, text/html
Details
2.05 KB, text/html
Details
569 bytes, text/xml
Details
545 bytes, text/html
Details
267 bytes, text/html
Details
1.31 KB, text/html
Details
911 bytes, text/html; charset=UTF-8
Details
40.23 KB, patch
bzbarsky
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
202 bytes, text/html
Details
using 7/8 build of apprunner

1) launch apprunner
2) launch editor
3) insert anchor

anchor icon doesn't get inserted.

all platforms
Assignee: cmanske → peterl
Summary: need to display anchor → CSS should allow display of normally-hidden tags, such as Named Anchor
I've already tried all kinds of CSS tricks to do this and have discussed it
with Peter, so I think it's best to just hand the bug to him.
For background, here's most of our last exchange:
Charles Manske wrote:
I can't seem to morph the "A" tag into something visible by any of your
suggestions. Here's what I settled on for "proof of concept"  testing, even
though it's not inline (this is in mozilla/editor/ui/composer/skin/Editor.css):
  a[name][href=""] {
    display: list-item;
    list-style-image: url(chrome://editor/skin/images/ED_Left.gif);
    list-style-position: inside;
    line-height: 10px;
    min-width: 10px;
  }
  Peter Linss wrote:

    Firstly, to make it apply to only <A> tags with the name attribute use:
"a[name]" for your selector (if you want to exclude hrefs that also
    have names, use: "a[name][href=""]").

    Charles Manske wrote:

     In the editor, we want to show a named anchor as a small image. To explore
this, I tried the following CSS:
     a {
       display: inline;
       width: 10px;
       height: 10px;
       border: 1px solid blue;
     }
    Inlines have special rules about respecting width and height, in short,
height doesn't work (use line-height) instead. I'm not sure about if
    width will work when the element is empty, you might try 'min-width'.
     or:
     a {
       display: list-item;
       list-style-image: url(chrome://editor/skin/images/ED_Left.gif);
     }
    This probably should work You should add 'list-style-position: inside' to
keep the image from appearing outside the element's margin.
    But again, I'm not sure if empty list-items get bullets... Also, display:
list-item will create a block level frame, I presume you really want an
    inline
     or:
     a {
       display: table; // or inline-table;
       background-image: url(chrome://editor/skin/images/ED_Left.gif);
     }

    This will probably only work in "standard mode", in "quirk mode" tables
don't draw backgrounds (it gets shoved into the cell). I'm also not
    sure if inline-table is working, but that's what you really want.

    I think what you really want is a bit of CSS3 that hasn't been implemented
yet (but I think we're going to have to real soon).
    a[name][href=""] {
      display: inline-block;    /* the new feature */
      width: 10px;
      height: 10px;
      background-image: url(...);
      border: 1px solid blue;
    }
One problem is that width doesn't apply to inline elements, so attempts to force
it will fail.  I see two solutions:

* wait for inline-block or inline-table
* use :before or :after and get joki to fix bug 5886 if you need it (i.e., check
in the fix attached to the bug)

And does [href=""] select elements without an href attribute?  I don't think it
should.

So I would think:

a[name]:before {
  content: url(....gif);
  border: 2px solid blue;
  }

a[name][href]:before {
  content: "";
  border: none;
  }

would be closer to what you want.  Also, since a elements with a name attribute
still enclose text, you probably want that text to still be visible, so I think
:before is a good choice.  (Note, however, that :before is really a first child,
not a previous sibling, so the style applied to the a applies to the :before.)
However, what if the document already has a :before pseudo-element for
anchors...?
I applied the patch for bug 5886. Using ":before" does indeed display an
image like we want. BUT it won't work because we don't allow selecting
generated content. The whole purpose of making the anchor visible is to allow
the user to click on it (thus selecting it) so they can delete it, copy
it, or bring up the property dialog for it.
Assignee: peterl → nisheeth
inline-block is in the style system. Waiting for Nisheeth and/or Troy to hook it
up to layout.
Status: NEW → ASSIGNED
Target Milestone: M9
Accepting bug and setting target milestone to M9...
Target Milestone: M9 → M10
Kipp has certain questions about the semantics of inline-block that he wants to
discuss with Peter once he gets back from the W3C CSS working group meeting.
We'll scope this out more for M10...
Summary: CSS should allow display of normally-hidden tags, such as Named Anchor → [RFE] Implement inline-block in layout
Target Milestone: M10 → M11
Updating summary and setting milestone to M11.  I need to work on this when I
get back from vacation.
Summary: [RFE] Implement inline-block in layout → [RFE] {css3} Implement inline-block in layout
[Adding a radar... Hmm, first time I've put in a {css3} radar...]
Is it really a good idea to be implementing a draft standard?  How close is CSS3
to proposed standard status?
Adding Akkana to cc.
The named anchor tag currently in the editor init page looks very bad on Linux
-- it draws on top of some of the text on that line and the next line.  Charley
says it doesn't do that on Windows, and that perhaps this has something to do
with the style not being fully implemented yet.  When working on this bug, it
might be useful to look at the named anchor in the editor init page on several
platforms and compare their presentations.
The style code is 100% cross platform, so the linux issue in not likely that.
Perhaps a font metrics issue?
I've split the Linux rendering issue off into bug 14474.
Target Milestone: M11 → M12
Charley, is this a beta blocker for you guys?  I'm moving it out to the M12
milestone for now...
We must solve the problem of displaying something for normally hidden elements
like Named Anchor, and there doesn't seem to be any other CSS way of doing that
*inline*, so I guess the answer is yes unless it is a lot of work.
Having reread the comments above, I don't really see how inline-block _will_
help, actually. I think David's use of generated content is probably best; why
can we not select generated content?

Alternatively, use this:
   a[name] { padding-left: 20px; background: url(...gif) 5px 50% no-repeat; }
There are some issues with 'background-position' on inline elements, though.
(I will split them off into another bug.)
Blocks: 14783
Depends on: 16515
Adding dependency to 16515 -- selection of generated content in the editor.
If we can't solve that bug, then we may need the inline-block CSS attribute to
represent hidden elements in the editor.
Depends on: 11771
No longer blocks: 14783
No longer depends on: 11771
Move non-PDT+ bugs to M13...
Target Milestone: M12 → M13
Keywords: css3
Target Milestone: M13 → M15
Moving non-blockers for beta to M15...
Summary: [RFE] {css3} Implement inline-block in layout → [RFE] Implement inline-block in layout
Just got word from charley that he is no longer blocked by this.  Moving off to 
M20.
Target Milestone: M15 → M20
This bug has been marked "future" because the original netscape engineer working 
on this is over-burdened. If you feel this is an error, that you or another
known resource will be working on this bug,or if it blocks your work in some way 
-- please attach your concern to the bug for reconsideration. 
Target Milestone: M20 → Future
I also ran into this. The problem is, that without inline-block, it's impossible 
to generate an inline element that has a certain (specified) width. As Mozilla 
supports other CSS3 properties, the question if or if not supporting CSS3 seems 
to be answered in the meantime. 
From my opinion there are two possible workarounds which should allow setting a 
predefined width: inline-table (which does not work) or using a table and 
setting the col-widths to the desired values (it's such an ugly workaround ...). 
I would like to hear, if there are other solutions, which I may have missed.
As per meeting with ChrisD today, taking QA.
QA Contact: sujay → py8ieh=bugzilla
*** Bug 118711 has been marked as a duplicate of this bug. ***
i've read (bug 3935, comment 108: ...I did disable 'inline-block' and
'inline-table'...) that we once had an implementation of this display style.
where there problems with that? if not, why was it removed?

IE (at least v6) does support 'display: inline-block' and i find it a very
elegant way for some design-ideas (though i wouldn't use it, unil it's supported
by some real browsers :-). i do know that this property is only in the cc3-wd,
so i've read it over and found that imho IE's implementation seems correct. and
i guess that it's not very hard to implement. 

i've also notived that there is a '-moz-inline-block' property that simply seems
to act like 'block' but does not honor the 'display-role: inline; display-model:
block-inside;' stuff.


the question is simply: are you going to wait till there is some css3-box-model
last call for comments or later, or is this just because of resource-shortage?
(last comment before: 2000-07-25)
resource shortage.

What used to be display:inline-block; is not display:-moz-inline-block; so you
can determine for yourself how badly broken it is (very).
CSS2.1 includes inline-block

inline-block is also needed for lots of form elements. These are today marked as
inline which is obviously incorrect since inline elements cannot have any width.

Increasing priority...
Priority: P3 → P2
Summary: [RFE] Implement inline-block in layout → Implement inline-block in layout
Form elements are "replaced elements" and CSS 2.1 doesn't say what to do with them.
Hixie, you wrote: "What used to be display:inline-block; is not
display:-moz-inline-block;", should "not" be "now"?

Reassigning to a better component.
Assignee: nisheeth → block-and-inline
Status: ASSIGNED → NEW
Component: Editor: Core → Layout: Block & Inline
Blocks: 184155
*** Bug 204010 has been marked as a duplicate of this bug. ***
Unless I misunderstand, inline-block allows you to nest block-level elements 
within, for example, a span.  The span continues to be displayed inline, but 
its contents are treated as a block-level "black box" that doesn't affect how 
the span is rendered (ie., you can imbed block-level constructs, such as UL).  
An example of a clean solution that would be very difficult without "inline-
block": http://www.bigtrouble.com/inline_block_1.htm (multiple columns, "shrink 
wrapped widths", centered horizontally).
(Sorry, try above link in IE6+, or Opera 7+)
*** Bug 217675 has been marked as a duplicate of this bug. ***
Blocks: 219873
Blocks: 163504
This test-case shows the inline-block (and -moz-inline-block) behaviour.  I
couldn't find a good test anywhere within Bugzilla, so I thought I'd place it
here for others to use.

BTW- any ideas on how to "fake" this behaviour using XBL?  I would imagine that
there's an element somewhere within Mozilla that affects layout like the <img>
tag, but can contain arbitrary HTML markup.
Attachment #135852 - Attachment mime type: text/plain → text/html
Adding a <br> inside the inline blocks to make it clearer. Also made sure that
the document validates to HTML 4.01 Strict
Attachment #135852 - Attachment is obsolete: true
Keywords: testcase
Thanks for the testcase update!  I've found a hacky (but virtually
indistiguishable) way to implement inline-block: use a <button>.  The <button>
tag is an arbitrary HTML container.  With the following CSS styles, the
"button"-esque nature of the element is removed, leaving an unstyled,
inline-block HTML element.  

Might there be a way to use the button rendering code to render <button> tags,
inline-blocks and inline-tables?  AFAIK, these should render with the same
layout rules.  Perhaps Hixie might know if this is a valid assumption.

button.E
{
	width: 100pt;
	border: 1px solid black;
	background: inherit;
	font: inherit;
	color: inherit;
	vertical-align: bottom;
}

See my new attachment for a demonstration of how this looks.  The only issue is
that the text within the <button> is unselectable.
I modified the test case again to use a hack that I found on the net. I already
lost the URI and it is not included in my Firebird history :'( The hack uses a
-moz-inline-box containing a block element. I guess this is basically what
button is doing.

Although one can achieve the desired layout using this technique it is really
not possible to use XBL to solve this problem. This really needs to be
supported natively in the layout engine.
Attachment #135929 - Attachment is obsolete: true
Attached file inline-block XBL hack
Using previously mentioned -moz-inline-box w/ block XBL hack.  Uses the XBL
from the previous attachment.
Using XBL and doing workarounds might be fun but this will not solve the real
issues here:

1. Will not support DOM Level 2 CSS
2. Not compatible with CSS2


I think I've done enough spamming of this bug so I'll keep quite now.
Flags: blocking1.6b?
Flags: blocking1.6b? → blocking1.6b-
Severity: normal → enhancement
Keywords: css2
It would be great if mozilla supported this. Opera does, Safari does, and hyatt
sez that WinIE does.
Blocks: 61902
Blocks: majorbugs
This is valid CSS that we don't support and IE of all things does. Please?
Attached file test case
Test case to show that only Opera has some native support for
'display:inline-block'.

Please don't spam this bug any futher with useless comments.
I was basing my remark about IE on the tests in comments #31 and #35, as well as
comment #25 - none of which anyone has repudiated. If these tests are wrong then
I apologise, if not I stand by my comment.
It would be nice to have support for inline-block - not only to call Firefox a
standards-compliant browser (it does not comply with CSS2.1), but also because
using handfuls of table cells etc. to do formatting is getting old.
Could somebody with enough knowledge about the core.layout part of the gecko engine point the rest 
of us to a document or something, that exlains the difficulty with implementing display:inline-block?

Aren't there already some elements that behave so similar (button, fieldset), that a proper 
implementation of display:inline-block seems achievable?
Please fix this bug, it's crucial for web apps and it works (almost) perfectly
in IE (no support in DIVs, yes support in SPAN and A).  If we want
Firefox/Mozilla to deliver on its promise this bug should not be allowed to be
alive for 5 years.
I think this bug needs to be changed from enchantment to normal. Firefox is
supposed to support CSS2.1 for starters. And this is also a CSS attribute that
would be very useful because it's implementation would reduce the need to use
hackish techniques such as using the float attribute to create tabbed interfaces
and other similar items.
Assignee: core.layout.block-and-inline → dbaron
Severity: enhancement → major
Priority: P2 → P1
Target Milestone: Future → mozilla1.9beta
As much as I enjoy using this bug to knock haughty firefox fanatics down a few 
pegs, I would much rather be able to use inline-block while coding. How about 
a status report, if nothing else?
Can anyone comment if this is fixable in the Firefox 1.1 timeframe? We have a
load of other new CSS features, and probably over two months for regression
checking. This is my only main gripe with Firefox/Mozilla's current level of CSS
support. The hackarounds necessary are up there with the solutions for some of
IE's hideous CSS problems.
It's not (correctly, at least).
I know nothing of the layout engine, but I still think this should be an easy
fix. An image is similar to a block element, as in, it's not text. Yet images
are already in-line by default and behave according to the specs.
Is treating a block element really that much more different than an image? Is it
to do with size calculation?
Whiteboard: [A2]
Flags: blocking1.8b2?
Flags: blocking1.8b2? → blocking1.8b2-
Flags: blocking-aviary1.1?
Asa Dotzler <asa@mozilla.org> has denied Steve England
<steve.england@gmail.com>'s request for blocking1.8b2: Bug 9458: Implement
inline-block in layout. 9th April 2005. I guess re-requesting blocking isn't
going to achieve anything.
Note to self (on the off chance that I'll ever even see it in between all the
comments on this bug that don't belong here):

In addition to being careful about shrink wrapping (which is the main reason
this hasn't been done yet), we also need to be careful about getting z-ordering
right (NS_FRAME_PAINT_LAYER_* stuff).  See http://www.w3.org/TR/CSS21/zindex.html .
No longer blocks: majorbugs
*** Bug 298384 has been marked as a duplicate of this bug. ***
*** Bug 299276 has been marked as a duplicate of this bug. ***
see comment 53, not fixable in this timeframe, moving on
Flags: blocking-aviary1.1? → blocking-aviary1.1-
*** Bug 301072 has been marked as a duplicate of this bug. ***
*** Bug 302637 has been marked as a duplicate of this bug. ***
just faked a tabulation class using the -moz-inline-box hack thus

span.tab
{
		display: -moz-inline-box;
		display: inline-block;
		width:	150px;
}

in html

<span class="tab">zeroth tab</span><spn class="tab">first tab</span><spn
class="tab">second tab</span>

works fine in opera,netscape,ie5,ie6 and firefox. though i havent attempted a 
pixel matching test, nor do i plan to. otherwise surely if there's a class that
emulates it already all you have to do is a string replace!?
-moz-inline-box is very different from inline-block.  -moz-inline-block is more
similar and less likely to cause problems in the future.
If you do a pixel matching test, you will find that just using a width is not
possible. In terms of height, padding, wrapping etc. spans behave much
differently to blocks. I've already tried using spans with padding etc. to
simulate in-line blocks, but have always found problems in different browsers.
(In reply to comment #63)
> -moz-inline-box is very different from inline-block.  -moz-inline-block is more
> similar and less likely to cause problems in the future.

tried -block but it inserts a carriage return after it, ergo: not inline

*** Bug 304681 has been marked as a duplicate of this bug. ***
Flags: blocking1.9a1?
For god's sake, please fix this NOW!  This bug actually has fan sites.  Firefox is notorious as being the ONE BROWSER unable to perform this simple function.
http://www.bigtrouble.com/inline_block.htm
Since this is the single most important RFE this should realy be checked in ASAP...
AFAIK this bug is dependent on the reflow changes dbaron is working on since more than a year, which is not quite ready, right?
But it sounds better to throw in a 80% finished patch and let a majority of people sqeeze out the last 20% of bugs generated by the patch instead of letting one single man complete this patch to 95% before checkin, which could take another year...
Anyone wants to give this bug a blocking1.9a1+ ?
just my 2 cents... and please dont burn me alive for moaning so much :D
(In reply to comment #68)
> Since this is the single most important RFE this should realy be checked in
> ASAP...

In Open Source communities, such demands sound rude. Please, consider never again writing them.

> Anyone wants to give this bug a blocking1.9a1+ ?

I doubt so. There is no patch nor any developer agreed to track the task. Dbaron said he don't know how long it'll take him, and if you think you can handle, go on, write down your proposal, with patches if possible.

> just my 2 cents...

If not, consider making a better use of your 2 cents, without rude demands and absolutely useless comments like those two above. This bug, like any other is not the place where people can share their frustrations and describe the reasons for why they so much want the bug to be fixed. Thank you for not replaing in this bug anymore if you don't have a patch proposal or other value to add. (my email is public if you want to discuss this comment)
Guys, keep the advocacy out of this thread. It's clearly on the developer's radar, so adding more "Fix this bug now!" posts to it just increases the signal to noise ratio and makes it harder to find useful information in the discussion (which is the point of Bugzilla). Take the advocacy to Mozillazine. I'm also going to request that you guys please read the Bugzilla etiquitte page.
https://bugzilla.mozilla.org/page.cgi?id=etiquette.html

That being said, since the next official build coming off the current trunk is looking to be version 3.0, it seems to me that we've got time to get this fixed correctly on the trunk. I'm sure David will create the necessary patch as soon as he feels it's ready and fairly regression-free. Now can we please leave discussion in this thread to useful posts only?
*** Bug 327658 has been marked as a duplicate of this bug. ***
Comment on attachment 212291 [details]
Javascript test for inline-block

The prompt is supposed to show the content of "display" in Firefox
Working fine in MSIE 6 if you want a reference of what I'm trying to do...
Comment on attachment 212291 [details]
Javascript test for inline-block

The prompt is supposed to show the content of "display" in Firefox
Working fine in MSIE 6 if you want a reference of what I'm trying to do...
> The prompt is supposed to show the content of "display" in Firefox
> Working fine in MSIE 6 if you want a reference of what I'm trying to do...

var divns6 = document.getElementsByTagName('tr');

getElementsByTagName: Returns a NodeList (...)
http://www.w3.org/TR/2000/REC-DOM-Level-2-Core-20001113/core.html#ID-A6C9094

"The items in the NodeList are accessible via an integral index, starting from 0."
http://www.w3.org/TR/2000/REC-DOM-Level-2-Core-20001113/core.html#ID-536297177

prompt(divns6[nObjet].style.display); 

Your testcase is not correct. Besides, this bugfile has a testcase and is about implementing inline-block.
Alias: inline-block
Flags: blocking1.9a1? → blocking1.9+
So one of my main concerns was shrink-wrapping, which is now fixed thanks to bug 300030.  Also, one of the frame construction issues I was worried about was filed as bug 337426 and is now fixed on the trunk.  I still need to look into comment 56.

With all this whining about how we absolutely must implement inline-block, I assume there's already a comprehensive test suite for it.  Could somebody point me to the URL so we don't have to spend a few days duplicating the effort that went into writing it?

A comprehensive test suite would cover:
 * dynamic changes to/from inline-block, to content inside of them, etc.
 * interaction with other properties (z-ordering)
 * putting various things inside of it
 * shrink-wrapping
 * any other conformance statements in CSS 2.1
and would generally be written as closely as possible to the CSS2.1 test suite authoring guidelines.

(And I really need the same for inline-table.)
Status: NEW → ASSIGNED
Depends on: reflow-refactor
Whiteboard: [A2] → [A2][patch]
(In reply to comment #76)
> With all this whining about how we absolutely must implement inline-block, I
> assume there's already a comprehensive test suite for it.  Could somebody point
> me to the URL so we don't have to spend a few days duplicating the effort that
> went into writing it?

Not quite a test suite but have a look at this article on alistapart: http://www.alistapart.com/articles/prettyaccessibleforms/ that gives you a good example of how it can be used and what it should do. Also, it comes with a piece of Javascript to tweak the DOM and fix the problem in Firefox so that may give you an idea of how to solve it.
So I definitely don't need to add support for -moz-inline-table, since that was actually a parse error before; I just need to add support for inline-table, which is what the next version of the patch will have.

I'm not sure whether I want to keep support for -moz-inline-block in addition to inline-block; authors who are using -moz-inline-block should generally be using inline-block as well, for other browsers, and we're better off encouraging those who aren't doing so already to do so.  So my inclination is to drop support for -moz-inline-block when adding support for inline-block.  (Good practice when using -moz-inline-block has always been "display:-moz-inline-block;display:inline-block".)
And it looks like nsIFrame::BuildDisplayListForChild should already handle the stacking context stuff correctly.
Attached patch patch (work in progress) (obsolete) — Splinter Review
This is the current state of my patch and tests.  Note that 6 of the tests fail:
 * 9458-width-1a fails because the shrink-wrapping behavior is actually incorrect, since I added aAvailableWidth to nsIFrame::ComputeSize
 * the 5 9458-zorder-* tests fail because of a vertical positioning issue that I need to figure out

I haven't started testing inline-table yet, but it needs the 9458-* tests copied, plus perhaps some additional tests of anonymous box construction (although we already have one, from bug 360065.
Attachment #248305 - Attachment is obsolete: true
Attached patch patch (work in progress) (obsolete) — Splinter Review
Attachment #248456 - Attachment is obsolete: true
Attached patch patch (work in progress) (obsolete) — Splinter Review
This has the tests fixed.  There's still one actual failure, and that's in one of the width tests.  I think we change "available width" during inline reflow in ways that we shouldn't, at least not for anything other than inlines and text.
Attachment #252006 - Attachment is obsolete: true
Attached patch patchSplinter Review
This fixes the test failure by making the availableWidth munging in nsLineLayout happen only for true inlines, and also adds more tests.
Attachment #252022 - Attachment is obsolete: true
Attachment #252099 - Flags: superreview?(bzbarsky)
Attachment #252099 - Flags: review?(bzbarsky)
It's worth noting that this bug also now depends on bug 163504 and bug 367247, because I've put related bug fixes to inline-block and inline-table in to those patches along with the uses.
Comment on attachment 252099 [details] [diff] [review]
patch

>diff -r 5df1530b6fe3 layout/base/nsCSSFrameConstructor.cpp
>@@ -6378,6 +6387,7 @@ nsCSSFrameConstructor::ConstructFrameByD
>   // XXX Ignore tables for the time being
>   if (aDisplay->IsBlockLevel() &&
>       aDisplay->mDisplay != NS_STYLE_DISPLAY_TABLE &&
>+      aDisplay->mDisplay != NS_STYLE_DISPLAY_INLINE_TABLE &&
>       aDisplay->IsScrollableOverflow() &&
>       !propagatedScrollToViewport) {
> 

Maybe that condition could be a method on nsStyleDisplay or something? E.g. part of IsScrollableOverflow()?

Followup bug if you agree.

Also a followup bug for all the NS_STYLE_DISPLAY_TABLE stuff in MathML code?  I don't know what behavior rbs wants for inline-table and all.

Do we need to add NS_STYLE_DISPLAY_INLINE_TABLE in the condition in PageBreakBefore()?  Then again, inline-table is not a block-level element, but I don't see where we restrict page-break to those...

>diff -r 5df1530b6fe3 layout/generic/nsFrame.cpp

>@@ -3773,6 +3773,7 @@ nsFrame::IsFrameTreeTooDeep(const nsHTML
>   // Absolute positioning causes |display->mDisplay| to be set to block,
>   // if needed.
>   return display->mDisplay == NS_STYLE_DISPLAY_BLOCK || 
>+         display->mDisplay == NS_STYLE_DISPLAY_INLINE_BLOCK || 
>          display->mDisplay == NS_STYLE_DISPLAY_LIST_ITEM ||
>          display->mDisplay == NS_STYLE_DISPLAY_TABLE_CELL;

Huh.  The function name in that "@@ .... @@" line is such a lie!

Should the condition here basically be something like IsBlockInside()?

That said, nsBlockFrame overrides this function, so what are the cases when we'll hit this code with a non-nsBlockFrame that has NS_STYLE_DISPLAY_INLINE_BLOCK?

> nsLineLayout.cpp

While I was looking at this stuff, it occurs to me that nsLineLayout::TreatFrameAsBlock should probably be a method on nsStyleDisplay...

I assume we want to make generated content kids of inline-block inline for now (that's what this patch does)?

I didn't review the tests in detail.  Let me know if you want me to.

r+sr=bzbarsky
Attachment #252099 - Flags: superreview?(bzbarsky)
Attachment #252099 - Flags: superreview+
Attachment #252099 - Flags: review?(bzbarsky)
Attachment #252099 - Flags: review+
I have been building with thei patch included for a bit now and noticed that the code in nsLineLayout needs some changed due to bug 367442.

mCOmputedWidth stuff.
I already have that fixed in my tree; it was too minor to bother commenting or including a new patch.
(though probably worth warning me about, so thanks)
(In reply to comment #87)
> Do we need to add NS_STYLE_DISPLAY_INLINE_TABLE in the condition in
> PageBreakBefore()?  Then again, inline-table is not a block-level element, but
> I don't see where we restrict page-break to those...

We really should restrict it to block-level elements; if we don't, that's a bug.

But I did notice while looking into this that I should fix IsSpecialFrameReframe as follows:

-    childIsBlock = display->IsBlockLevel() || IsTableRelated(display->mDisplay, PR_TRUE);
+    childIsBlock = display->IsBlockLevel();

since anonymous table objects force an inline-table when they're within an inline (as does inline-table itself, which is IsTableRelated).

> >diff -r 5df1530b6fe3 layout/generic/nsFrame.cpp
> 
> >@@ -3773,6 +3773,7 @@ nsFrame::IsFrameTreeTooDeep(const nsHTML
> >   // Absolute positioning causes |display->mDisplay| to be set to block,
> >   // if needed.
> >   return display->mDisplay == NS_STYLE_DISPLAY_BLOCK || 
> >+         display->mDisplay == NS_STYLE_DISPLAY_INLINE_BLOCK || 
> >          display->mDisplay == NS_STYLE_DISPLAY_LIST_ITEM ||
> >          display->mDisplay == NS_STYLE_DISPLAY_TABLE_CELL;
> 
> Huh.  The function name in that "@@ .... @@" line is such a lie!
> 
> Should the condition here basically be something like IsBlockInside()?

Yeah, I should fix that once I land IsBlockInside in the next patch...

> That said, nsBlockFrame overrides this function, so what are the cases when
> we'll hit this code with a non-nsBlockFrame that has
> NS_STYLE_DISPLAY_INLINE_BLOCK?

Probably pretty low.  I suppose it should perhaps even just return PR_FALSE so that we don't say that something that can't be a containing block is one.

> > nsLineLayout.cpp
> 
> While I was looking at this stuff, it occurs to me that
> nsLineLayout::TreatFrameAsBlock should probably be a method on
> nsStyleDisplay...

I'm not so sure, since it has only one caller and doesn't seem generally useful (it's a rather odd combination).

> I assume we want to make generated content kids of inline-block inline for now
> (that's what this patch does)?

Hrm.  No, actually.  12.1 of CSS2.1 doesn't have those restrictions anymore, so we should at least allow block inside inline-block.  I'll fix that (in nsRuleNode::ComputeDisplayData).

> I didn't review the tests in detail.  Let me know if you want me to.

No need.
(In reply to comment #91)
> (In reply to comment #87)
> > Should the condition here basically be something like IsBlockInside()?
> 
> Yeah, I should fix that once I land IsBlockInside in the next patch...
> 
> > That said, nsBlockFrame overrides this function, so what are the cases when
> > we'll hit this code with a non-nsBlockFrame that has
> > NS_STYLE_DISPLAY_INLINE_BLOCK?
> 
> Probably pretty low.  I suppose it should perhaps even just return PR_FALSE so
> that we don't say that something that can't be a containing block is one.

In the patch to bug 367247 I decided to make it do the latter -- just return PR_FALSE, and add an IsContainingBlock to nsTableCellFrame.
(In reply to comment #92)
> In the patch to bug 367247 I decided to make it do the latter -- just return
> PR_FALSE, and add an IsContainingBlock to nsTableCellFrame.

I had to back the nsFrame part of that out due to orange; probably need to add overrides on nsViewportFrame / nsHTMLFrame or something similar.
> Yeah, I should fix that once I land IsBlockInside in the next patch...

Did this ever get a bug report?  And the MathML stuff?

Also, there's some code (or rather misleading comments) in ProcessInlineChildren() that we talked about going away:

12640     // recompute allKidsInline to take into account new child frames
12641     // XXX we DON'T do this yet because anonymous table children should

etc.
Yeah, I haven't resolved this yet because I haven't had a chance to file all the necessary followup bugs.
Depends on: 369547
(In reply to comment #87)
> Followup bug if you agree.

Filed bug 371446.

> Also a followup bug for all the NS_STYLE_DISPLAY_TABLE stuff in MathML code?  I
> don't know what behavior rbs wants for inline-table and all.

Filed bug 371448.

> Do we need to add NS_STYLE_DISPLAY_INLINE_TABLE in the condition in
> PageBreakBefore()?  Then again, inline-table is not a block-level element, but
> I don't see where we restrict page-break to those...

Filed bug 371449.

> >@@ -3773,6 +3773,7 @@ nsFrame::IsFrameTreeTooDeep(const nsHTML
> >   // Absolute positioning causes |display->mDisplay| to be set to block,
> >   // if needed.
> >   return display->mDisplay == NS_STYLE_DISPLAY_BLOCK || 
> >+         display->mDisplay == NS_STYLE_DISPLAY_INLINE_BLOCK || 
> >          display->mDisplay == NS_STYLE_DISPLAY_LIST_ITEM ||
> >          display->mDisplay == NS_STYLE_DISPLAY_TABLE_CELL;
> 
> Huh.  The function name in that "@@ .... @@" line is such a lie!
> 
> Should the condition here basically be something like IsBlockInside()?

Filed bug 371450.

> That said, nsBlockFrame overrides this function, so what are the cases when
> we'll hit this code with a non-nsBlockFrame that has
> NS_STYLE_DISPLAY_INLINE_BLOCK?

Filed bug 371452.


Checked in to trunk on 2007-01-27.  Marking fixed.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
Flags: in-testsuite? → in-testsuite+
Depends on: 373625
This was documented a while ago.
Depends on: 398144
Depends on: 398797
(In reply to comment #100)
> https://bugzilla.mozilla.org/show_bug.cgi?id=9458 MUST be reopened

Eugen, this bug is fixed in test builds of Firefox 3 (codenamed Gran Paradiso). Next time, you might want to test your attachment in the latest test release before disputing a “RESOLVED FIXED” status.

You might want to also use valid HTML in your attachments; your current attachment is invalid and triggers quirks mode.
Jonathan. Will developers of FF leave FFv2.0 unfixed and apply only security patches to it?
Yes, the stable release is only getting security and stability patches.  It's not getting feature work (like this bug).
Depends on: 402338
No longer depends on: 402338
Depends on: 421397
Should it not be REOPENED, as it depends on two bugs still OPEN?
No, bugs are not reopened for regressions.
Depends on: 427638
Blocks: 361352
Restrict Comments: true
Depends on: 728891
You need to log in before you can comment on or make changes to this bug.