326932 - Triple-click paragraph selection doesn't work correctly in paragraphs containing form controls

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[Uri Bernstein]

In a paragraph which contains an <input type="text">, triple-clicking before the input control selects nothing. Triple-clicking after the (last) control selects only the part of the paragraph after the control.

Triple-click should in both cases select the entire paragraph.

Thanks to Martijn, which originally discovered this for <isindex>.

Comment 1

[Uri Bernstein]

Attachment: testcase

Comment 2

[Martijn Wargers (dead)]

Well, iirc, Ria originally discovered this for <isindex>, I was merely passing it through :)

Comment 3

[Steve England [:stevee]]

This behaviour has changed since 1.5. With 1.5.0.1 double-clicking any of the text results in the _whole_ line of text being selected (both sides of the input box)

Comment 4

[Uri Bernstein]

(In reply to comment #3)
> This behaviour has changed since 1.5. With 1.5.0.1 double-clicking any of the
> text results in the _whole_ line of text being selected (both sides of the
> input box)

Steve, this is a trunk-only bug (since bug 32807 was only fixed on the trunk).

Comment 5

[Uri Bernstein]

Attachment: testcase 2

This testcase demonstrates that the problem is not limited to text inputs, but affects all form controls (in this way or another).

Comment 6

[Uri Bernstein]

Attachment: patch

Some form controls are block frames, and some contain (anonymous) block frames. Since we don't want to form controls to be paragraph separators, this patch simply skips them (without drilling into them) when looking for block frames.

Is there a more general solution I can employ here? Testing for form controls is a bit ugly.

This patch does *not* fix the <isindex> case, which is apparently a different problem (although the isindex's content is the parent content of the HRs' content, trying to find the HR content inside the isindex content using IndexOf() returns -1).

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

You attached the wrong file.

Depending on how you were doing this, you may want to test line->IsBlock() instead of looking for block frames per se.

Comment 8

[Uri Bernstein]

Attachment: patch

Oops. This is the original patch I meant to attach. I'll look into roc's suggestion and request a review (on this patch or a modified one) later.

Comment 9

[Uri Bernstein]

Comment on attachment 213007 [details] [diff] [review]
patch

Other methods for checking for blockness didn't quite work for me, so requesting review on this.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 213007 [details] [diff] [review]
patch

Please add a comment

// XXX we really need a way to determine whether a frame is inline-level

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I wonder if you could rewrite FindBlockFrameOrBR to iterate over lines with a separate function for iterating within lines. Then you could know whether you're looking at a line of inlines (even if they are inline blocks), and eDirPrevious could be faster since the lines are a doubly linked list. Did you try this?

Comment 12

[Uri Bernstein]

Checked in, with the extra comment:

Checking in layout/generic/nsFrame.cpp;
/cvsroot/mozilla/layout/generic/nsFrame.cpp,v  <--  nsFrame.cpp
new revision: 3.622; previous revision: 3.621
done

Comment 13

[Boris Zbarsky [:bzbarsky]]

Do you think we could have a reasonable IsFrameOfType bit for this?  That is, is this something we might need for non-form-control frames too?  Say for inline-block?

Comment 14

[Uri Bernstein]

(In reply to comment #11)
> I wonder if you could rewrite FindBlockFrameOrBR to iterate over lines with a
> separate function for iterating within lines. Then you could know whether
> you're looking at a line of inlines (even if they are inline blocks), and
> eDirPrevious could be faster since the lines are a doubly linked list. Did you
> try this?
> 

Oops - I somehow missed this comment before checking in, only noticed it now.

No, I haven't tried this. I'll try to do this soon.

Comment 15

[Uri Bernstein]

(In reply to comment #13)
> Do you think we could have a reasonable IsFrameOfType bit for this?  That is,
> is this something we might need for non-form-control frames too?  Say for
> inline-block?
 
This was directed to roc, right?

Me, I'd love to have a method which would tell me whether a frame behaves like an inline "from the outside". I was pretty surprised not to find one.
But I can't say much intelligently about an implementation of such a method.

Comment 16

[Boris Zbarsky [:bzbarsky]]

> This was directed to roc, right?

Well... it was directed at anyone who cares to answer.

> I'd love to have a method which would tell me whether a frame behaves like
> an inline "from the outside"

I believe we want such things anyway.  So we should add it... dbaron, do we have a bug on doing that, do you know?