314878 - [FIX]Remove QIs to nsIDOMHTMLSelectElement before calling WipeContainingBlock

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Boris Zbarsky [:bzbarsky]]

See discussion in bug 313496.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Like so

Comment 2

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 201717 [details] [diff] [review]
Like so

>   // If we don't have a block within an inline, just return false.
>   // XXX We should be more careful about |aFrame| being something
>-  // constructed by tag name (see the SELECT check all callers currenly
>-  // do).
>+  // constructed by tag name; it might have a "block-like" frame
>+  // that's not an area of block and not have a block display....

area *or* block

>+  nsIAtom* frameType = aFrame->GetType();
>   if (NS_STYLE_DISPLAY_INLINE != aFrame->GetStyleDisplay()->mDisplay ||
>+      frameType == nsLayoutAtoms::blockFrame ||
>+      frameType == nsLayoutAtoms::areaFrame ||

Why not do the check the other way around:  skip the display check, and just check frameType != nsLayoutAtoms::inlineFrame && frameType != nsLayoutAtoms::positionedInlineFrame && frameType != nsLayoutAtoms::lineFrame.

(Not sure about the lineFrame, though.)

Comment 3

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

It looks like block-within-inline splitting is done in ConstructInline, which is called only for inlineFrame and positionedInlineFrame types.  Not sure what bugs we have with {ib} and :first-line, though.

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Though nsCSSFrameConstructor::WrapFramesInFirstLineFrame calls IsInlineFrame, so I think you probably would want to check lineFrame type.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch with that change

<sigh>.   I even talked about that, then forgot to do it...

Comment 6

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk.