[webkit-reviews] review denied: [Bug 26342] Absolutely positioned HTML element within foreignObject of absolutely positioned SVG crashes Safari : [Attachment 39760] make the foreignobject the containing block for the abs positioned child block if no appropriate containing block found

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Charles Wei
<charles.wei at torchmobile.com.cn>'s request for review:
Bug 26342: Absolutely positioned HTML element within foreignObject of
absolutely positioned SVG crashes Safari
https://bugs.webkit.org/show_bug.cgi?id=26342

Attachment 39760: make the foreignobject the containing block for the abs
positioned child block if no appropriate containing block found
https://bugs.webkit.org/attachment.cgi?id=39760&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
I think that's still not quite right.  I think you want to change the while
condition instead:

	while (o && (o->style()->position() == StaticPosition || (o->isInline()
&& !o->isReplaced())) && !o->isRenderView() && !(o->hasTransform() &&
o->isRenderBlock())) {

Admittedly, I think that condition would be much clearer written as a bunch of
if statements in the while body, each calling "break" when true.

while (o)) {
  if (o->isRenderView())
      break;
  if (o->hasTransform() && o->isRenderBlock())
      break;
  if (o->style()->position() != StaticPosition && (o->isReplaced() &&
!o->isInline())
      break;
}

Not something you'd have to change, but looking at it, that loop is pretty
ugly/hard-to-read.

o->node()->hasTagName(SVGNames::foreignObjectTag)
is probably a bad check too.  We should probably add a isForeignObject() to
RenderObject instead.

Anyway, another way to add what you want would be to just add:
if (o->isForeignObject())
   break;
instead of needing to edit the existing if or while condition.

If this test is just trying to test the crash it can just be a dumpAsText()
test.  If you need to test the positioning of the text, then you might need to
make it render tree test as you have.