785324 - Setting innerHTML on empty element selects the text in it

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[Nikolay Atanasov]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.1 (KHTML, like Gecko) Chrome/21.0.1180.79 Safari/537.1

Steps to reproduce:

Run this code in browser

<div style="float:left;width:100px;height:100px;border:1px solid black;" onclick="this.innerHTML = 'TEST';"></div>

Click on the div. 


Actual results:

The text is selected. 


Expected results:

The text should not be selected

Comment 1

[Alice0775 White]

Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/c7101dec8deb
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111220 Firefox/11.0a1 ID:20111220083550
Bad:
http://hg.mozilla.org/mozilla-central/rev/a8506ab2c654
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111220 Firefox/11.0a1 ID:20111220085450
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c7101dec8deb&tochange=a8506ab2c654


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/feaccb6a4c35
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111219 Firefox/11.0a1 ID:20111219235256
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/0aa9c3a5b7e1
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111219 Firefox/11.0a1 ID:20111220011653
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=feaccb6a4c35&tochange=0aa9c3a5b7e1

Suspected:Bug 619273

Comment 2

[Mats Palmgren (inactive)]

nsFrame::HandlePress calls HandleClick with the content node
and start/end offsets that it gets from GetContentOffsetsFromPoint:
GetContentOffsetsFromPoint
  GetSelectionClosestFrame
    GetSelectionClosestFrameForBlock
http://hg.mozilla.org/mozilla-central/annotate/35431a5588e0/layout/generic/nsFrame.cpp#l3354
here we find the empty DIV and return FrameTarget::Null()

back in GetSelectionClosestFrame
http://hg.mozilla.org/mozilla-central/annotate/35431a5588e0/layout/generic/nsFrame.cpp#l3427
we fall through, finds that |kid| is null, and return
FrameTarget(aFrame, false, false)

back in GetContentOffsetsFromPoint
http://hg.mozilla.org/mozilla-central/annotate/35431a5588e0/layout/generic/nsFrame.cpp#l3549
we fall through to the last line that returns
CalcContentOffsetsFromFramePoint which returns DIV 0,1

HandleClick then selects everything in the DIV 0,1 range.

I'm not sure why bug 619273 have any effect on this stuff, I'm guessing
this bug was always present and that bug 619273 just made it visible.

Comment 3

[Mats Palmgren (inactive)]

Attachment: fix

Taking the same path as for editable content here seems reasonable.
So we'll return earlier with offset==secondaryOffset==0 here:
http://hg.mozilla.org/mozilla-central/annotate/35431a5588e0/layout/generic/nsFrame.cpp#l3552

https://tbpl.mozilla.org/?tree=Try&rev=3fec7d47fb1d

Comment 4

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8bdeda83f7b1

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8bdeda83f7b1