Bug 39315 – selections in margins wrong with div followed by a span

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Ojan Vafai]

Created attachment 56409 [details]
test case

See test case. Originally seen on http://www.lewiz.org/2010/05/chrome-incognito-tracks-visited-sites.html.

[Robin Qiu]

hi Ojan, I have investigated this bug. It's cause by this line:
http://trac.webkit.org/browser/trunk/WebCore/rendering/RenderBlock.cpp#L3783
here, childBox->frameRect().bottom() is used, but, I think, childBox->maxBottomMargin(true) is more suitable. In that case, webkit acts similarly with FF.

I would like to hear other people before I prepare a patch for this bug.

[Robin Qiu]

It seems that FF use half of the margin as the boundary of this two elements. 
Should I change this code 
if (isChildHitTestCandidate(childBox) && contentsY < childBox->frameRect().bottom())
to 
if (isChildHitTestCandidate(childBox) && contentsY < childBox->frameRect().bottom() +  childBox->maxBottomMargin(true) / 2)
?

Is there any specification defines this?

[Robin Qiu]

Created attachment 57730 [details]
I have to push this bug by committing a patch. :)

[Ojan Vafai]

Comment on attachment 57730 [details]
I have to push this bug by committing a patch. :)

(In reply to comment #2)
> It seems that FF use half of the margin as the boundary of this two elements. 
> Should I change this code 
> if (isChildHitTestCandidate(childBox) && contentsY < childBox->frameRect().bottom())
> to 
> if (isChildHitTestCandidate(childBox) && contentsY < childBox->frameRect().bottom() +  childBox->maxBottomMargin(true) / 2)
> ?

How does this fix the bug? Doesn't a similar bug still happen? Instead of happening immediately after the div in the test case, it happens halfway through the margin of the div in the test case. In other words, when you select past the half-way point, but still above the last line, the last line gets selected and it shouldn't. Am I wrong? Does this patch fix that case?

> Is there any specification defines this?

There is not a specification for this. It is up to the user-agent. We've tried to match IE/FF where it makes sense to avoid making an arbitrarily different decision for no reason and to match general user-expectations (that assumes users are used to the IE/FF behavior). It's not clear to me whether we should match Firefox here or not, but in either case, it's a separate bug.

[Robin Qiu]

(In reply to comment #4)
I don't think this bug is a 'real' bug, becuase no spec indicates how to deal with this case.  I committed this patch just becuase I thought FF's behavior are more reasonable than WebKit. After patching this patch, WebKit will act similarly with FF.

All in all, this is not a technical bug, I think it can be closed without any modification.

[Ojan Vafai]

(In reply to comment #5)
> (In reply to comment #4)
> I don't think this bug is a 'real' bug, becuase no spec indicates how to deal with this case.  I committed this patch just becuase I thought FF's behavior are more reasonable than WebKit. After patching this patch, WebKit will act similarly with FF.
> 
> All in all, this is not a technical bug, I think it can be closed without any modification.

Actually, it looks like the original bug is fixed. Not sure which revision it was. Try the test case in a WebKit nightly versus a Safari 4 or Chrome 5. It works correctly in the nightly.