Oops. The new UI can alter page layout in some cases. EG, "foo<object/>bar" will normally display a working plugin in-line, but with the new plugin-problem UI when the plugin crashes (or is missing/blocked/etc), the object essentially becomes a block element. (foo, object, and bar on separate lines).

Grr. This was giving me fits with weird layout problems. Then I rebuilt my tree, and it started working better. I hope the brokenness (as opposed to the workingness) was just a glitch. Anyway, this seems to fix the problem as stated in this bug. However, now I have a new problem -- the code in browser.js was using pluginElement.getBoundingClientRect() to determine the size of the plugin (so it could see if the binding's content was too small to fully show). Now this call is returning the expected width, but not height. For example, when crashing the test plugin sized to 300x200, the binding's .mainBox size is 310x210, but the plugin's rect is 310x15. [and the plugin's .clientHeight / .clientWidth is 0x0, same as before]. Note that the 310x210 vs 300x200 difference seems to be a result of bug 446693. So, I'm stumped right now as to how to make this code in browser.js start working again.

I don't know what your markup is for the testcase you talk about in comment 1, but it sounds like you have something like an <object> with display:inline which the binding is attached to. The bounding client rect for that will have a height equal to the font-size, more or less, unless you end up with a block-inside-inline split. The key part here is that when we fall back from the <object>, we end up treating it as a non-replaced box, and creating a frame for it by display. This is in fact the right thing to do when showing the alternate content, but when showing the binding you want to keep it at its "sized box" size (and effectively treat as either block or inline-block, depending), right? For images we handle this by not falling back to text when we want to preserve the size in the layout... I'm not sure what a good solution is for plug-ins yet (at least in part because I'm not quite sure what the envisioned behavior of all the cases when the plugin binding gets attached is yet). If you _really_ want the new UI to not alter the page layout (compared to when the plug-in is in the same state but the UI is not being shown), then you can't extract the information about how big the plug-in would have been if it were in a different state (which is what it sounds like you want) very easily...

Is the underlying problem that you have markup like <object width="200" height"100" ...> and when the crash UI is shown, those attributes cease to map to CSS 'width' and 'height'?

They get mapped into _specified_ (and in our impl computed) style no matter what, but when fallback occurs the <object> gets an nsInlineFrame (unless it has a non-default display style), which ignores those properties for layout purposes.

Aren't we giving it display:inline-block in the crashed state, in which case those styles should be applied?

We're not, at least with the patch in this bug. We're giving the anonymous content of the binding an inline-block display, but not the <object> itself.

Well then, why don't we fix that?

It's not clear to me how to fix that, because I can't tell what the desired behavior is. Hence comment 2...

(In reply to comment #2) > I don't know what your markup is for the testcase you talk about in comment 1 Just a simple HTML document with: begin<object type="application/lolcat" width="200" height="133"><param name="pluginurl" value="http://mozilla.com"/></object>end Or the same thing as a working Flash plugin, so that killing mozilla-runtime gives you the crashed-plugin UI instead of the missing-plugin UI: begin<object type="application/x-shockwave-flash" data="http://sadtrombone.com/sad_trombone.swf" width="200" height="133"><param name="bgcolor" value="#0000ff"/></object>end The "begin" and "end" text should be at the same vertical position, but are not when our UI is shown. > The key part here is that when we fall back from > the <object>, we end up treating it as a non-replaced box, and creating a frame > for it by display. I'm not sure exactly what that means... > This is in fact the right thing to do when showing the > alternate content, but when showing the binding you want to keep it at its > "sized box" size (and effectively treat as either block or inline-block, > depending), right? The basic goal is that when the binding is shown, it should exactly replace the box where the plugin's contents would be shown -- don't change the page's layout. The exceptions to this are: * There are some legacy styles that attempt to make an unsized plugin change to 240x200. Not sure if we want to keep that or not. http://mxr.mozilla.org/mozilla1.9.2/source/toolkit/themes/winstripe/mozapps/plugins/missingPlugin.css * The existing code uses the page's fallback content (instead of our missing-plugin UI) in some cases, that seems correct but I've not considered it in detail yet. The other plugin states (crashed/blocked/disabled) seem to have inherited the same behavior -- I think that's wrong, but I'm not sure if the fallback content on real pages does graceful degradation, or just says "flash required". Fodder for another bug... * If the plugin's box is too small to fully show the binding's UI, we'll change the binding to visibility: hidden and show a notification bar instead. I implemented this for the plugin-crashed case, but the other cases (missing/blocked/disabled) just do whatever it is they've always done (I plan to revist that soon). There might be other edge cases where we're not doing the right thing; I'm not sure if anyone's ever looked closely at making this UI work flawlessly since to date it's not common for users to see it. Of course there are no existing tests for this stuff, though I'm collecting things to check for in new tests. > If you _really_ want the new UI to not alter the page layout (compared to when > the plug-in is in the same state but the UI is not being shown), then you can't > extract the information about how big the plug-in would have been if it were in > a different state (which is what it sounds like you want) very easily... That sounds like what I want. :-(

It seems to me that if we had a style rule that made the <object> display:inline-block when the crashed UI is shown, things would work a lot better, maybe "good enough". If we absolutely have to have the crashed UI always sized to the plugin, regardless of the cost, then the approach where we add an anonymous child of an nsObjectFrame is probably best.

> The "begin" and "end" text should be at the same vertical position Unless the <object> in question is styled with display:block by the page, yes. I understand that. > I'm not sure exactly what that means... When an <object> falls back, we essentially replace it with a <span> but apply all the CSS rules that used to be applied to the <object> with that <span>. That's more or less what the "non-replaced box" thing means. The idea is that the kids of the object are then rendered as kids of the <span> to show the fallback content... except in the crashed-plugin case you don't want to show those kids, in fact. > There are some legacy styles that attempt to make an unsized plugin change to > 240x200 I see no reason to maintain that (or at least to put more effort into doing so). > The existing code uses the page's fallback content Right. This is the behavior I describe above. The behavior here is "by design"; dunno about "correct". > It seems to me that if we had a style rule that made the <object> > display:inline-block That wouldn't work if the page explicitly styles it as display:inline (unless we made our rule !important, and then it wouldn't work if the page styles it as display:block). > then the approach where we add an anonymous child of an nsObjectFrame is > probably best Yep. That's what we do for images when we have to preserve the size at all costs...

(In reply to comment #11) > That wouldn't work if the page explicitly styles it as display:inline Indeed. The question is whether there are enough of such cases that it's worth adding complexity to handle them.

Sure. If we're ok with "losing" in the rare case, then just styling the object as inline-block in UA.css when doing the binding stuff is the way to go.

Adds inline-block to the plugin element itself. I also had to add "vertical-align: bottom", so that leading/trailing content is aligned with the bottom of the plugin-problem box (as it is when the plugin is working ok), instead of the top. Along the way, I discovered some related style and pseudoclasses I didn't know existed at http://mxr.mozilla.org/mozilla-central/source/layout/style/html.css#542, which I think is actually causing this. Will attach a newer patch dealing with the latter next.

Grr. The stuff in html.css was a red-herring, doesn't make a difference. The problem I'm having now (with the last patch) is that things were still shifting a few pixels vertically. The problem is more obvious with: <div style="border: 1px solid black; font-size: 150px">XXX<object/></div> The normal plugin is bottom-aligned with the font's baseline, the crashed-UI isn't and the <div> shrinks. I've fiddled with various style tweaks, but I think I just don't understand what's going on. :( The main reason I'd like to get this working precisely right is that I want to write tests to make sure that the plugin-problem UI doesn't change page layout (other than the accepted problem from comment 12/13), and that this seems like a common enough case that sites relying on pixel-precise layout could change in odd ways when plugins crash. Still, this is better than what's in the tree right now...

> but I think I just don't understand what's going on. For a normal plug-in, the baseline is located at the bottom of the border-box. It is then by default baseline-aligned (though the page can change this). The result is that the bottom of the border-box lines up with the text baseline. You don't really want to change the vertical-align of the plug-in element itself, but you DO want to change where its baseline is, I think. Thinking about how best to accomplish that.

Hmm. What happens if you set overflow:hidden on the <object> when your binding is applied?

Yay, bz's suggestion works! The explanation is http://www.w3.org/TR/CSS2/visudet.html#line-height, bottom paragraph of the page... "The baseline of an 'inline-block' is the baseline of its last line box in the normal flow, unless it has either no in-flow line boxes or if its 'overflow' property has a computed value other than 'visible', in which case the baseline is the bottom margin edge." Which is exactly what we want here. It's a little unclear exactly what what being used as the baseline when just setting "display: inline-block" (without modifying vertical-align or overflow). It should be coming from the <vbox>, which in turn might be getting it from the first <spacer> before flex effects? Sounds plausible enough to me, given that we're in XUL+HTML land.

Pushed http://hg.mozilla.org/mozilla-central/rev/67abe74452a6

Blanket approval for Lorentz merge to mozilla-1.9.2 a=beltzner for 1.9.2.4 - please make sure to mark status1.9.2:.4-fixed

Merged into 1.9.2 at http://hg.mozilla.org/releases/mozilla-1.9.2/rev/84ba4d805430