Bug 39315

Summary: selections in margins wrong with div followed by a span
Product: WebKit Reporter: Ojan Vafai <ojan>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: charles.wei, darin, eric, hyatt, robin.qiu, staikos
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
test case
none
I have to push this bug by committing a patch. :) ojan: review-

Description Ojan Vafai 2010-05-18 13:50:25 PDT
Created attachment 56409 [details]
test case

See test case. Originally seen on http://www.lewiz.org/2010/05/chrome-incognito-tracks-visited-sites.html.
Comment 1 Robin Qiu 2010-05-28 01:28:42 PDT
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.
Comment 2 Robin Qiu 2010-05-31 03:47:09 PDT
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?
Comment 3 Robin Qiu 2010-06-02 21:06:02 PDT
Created attachment 57730 [details]
I have to push this bug by committing a patch. :)
Comment 4 Ojan Vafai 2010-06-08 17:32:10 PDT
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.
Comment 5 Robin Qiu 2010-06-08 21:22:30 PDT
(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.
Comment 6 Ojan Vafai 2010-06-09 16:31:13 PDT
(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.