Bug 90462 – [BlackBerry] Touch hold on input field does trigger selection for some special cases

• Status: RESOLVED
• Resolution: INVALID
• Component: WebKit BlackBerry

[Yongxin Dai]

Node::canStartSelection() can return false on the host node of the selectable input text field for some special cases.

[Yongxin Dai]

Created attachment 150620 [details]
Patch

[Antonio Gomes]

Comment on attachment 150620 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150620&action=review

could you confirm the original problem fixed by the code you are changing does not regress?

> Source/WebKit/blackberry/ChangeLog:6
> +        Reviewed by Antonio Gomes.

I have not reviewed it :)

Tools can fill it up for you

[Yongxin Dai]

Created attachment 150622 [details]
Patch

[Yongxin Dai]

Created attachment 150631 [details]
Patch

[Rob Buis]

Comment on attachment 150631 [details]
Patch

LGTM.

[Yongxin Dai]

I can confirm the original problem fixed by the code. I tested the fix with various text input fields in past 2 days. I haven't found problem yet.


(
(In reply to comment #2)
> (From update of attachment 150620 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=150620&action=review
> 
> could you confirm the original problem fixed by the code you are changing does not regress?
> 
> > Source/WebKit/blackberry/ChangeLog:6
> > +        Reviewed by Antonio Gomes.
> 
> I have not reviewed it :)
> 
> Tools can fill it up for you

[Rob Buis]

Comment on attachment 150631 [details]
Patch

It turns out the question Antonio asked still needs to be answered. Also the ChangeLog can be cleaned up a bit more, so far Antonio did not really review, so we should not mention him in the ChangeLog.

[Yongxin Dai]

Hi Antonio,
I must misunderstood "original problem" in my last comment. What is the original problem you talked about?

[Yongxin Dai]

Created attachment 150652 [details]
Patch

[Antonio Gomes]

(In reply to comment #8)
> Hi Antonio,
> I must misunderstood "original problem" in my last comment. What is the original problem you talked about?

I mean, without your change, do a git blame to these lines you are changing, and check what was the problem it was trying to fix, when we decided to not work with shadow content. Basically, I am wondering if changing it back to how it was regresses the original issue fixed by these lines.

[Rob Buis]

Comment on attachment 150652 [details]
Patch

LGTM.

[Yongxin Dai]

The Original PR is #127814. It just said that text fields from fat finger should all ASSERT(!node->isInShadowTree()). But specific reason had not been mentioned.

[Mike Fenton]

Invalid, the field should not be read only in the first place.

Investigation is continuing, but this is not the desired approach.