609583 - Text selection functionality doesn't work

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect)

Description

[Egor Starkov]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.10) Gecko/20100914 Firefox/3.5
Build Identifier: 

PRECONDITION: pref("browser.tabs.remote", true);
I work on extension that allows fennec to make text selection and found that selection functionality doesn't work properly at the moment because layout jumps away when I start to select (send mousedown and then mousemove events). nsFrame::HandleDrag function handles the selection and also keeps it always visible by calling StartAutoScrollTimer. The problem is that real scrollport is not changed in current e10s implementatio and always stays on top, AutoScroll feature makes layout jump and changes scroll position.
Can we workaround or disable this selection scroll feature until real scrollport will be fixed?

Reproducible: Always

Comment 1

[Egor Starkov]

Attachment: workaround patch v1

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I don't have enough layout background to review your patch, and I barely understand the problem you're describing.  Is your extension running in the chrome process?  (I think yes.)

Initially though, using mousedown+mousemove in your extensions to select text seems off to me, whether in the chrome or content process.  What are you trying to do?  Do we have other text-selection APIs that would work better?

Comment 3

[Benjamin Smedberg]

Comment on attachment 488201 [details] [diff] [review]
workaround patch v1

Use XRE_GetProcessType instead of nsIXULRuntime (much cheaper!), and please request the next review from roc.

Comment 4

[Egor Starkov]

My Fennec extension works in chrome process and it sends artificial "mousedown + mousemove" events to imitate text selection in firefox. The thing is AutoScroll feature that changes scroll to keep selection always visible doesn't work now because real scrollport is not updated in electrolysis. So I suggest to disable this feature for now until scrollport issue will be fixed. Patch is proposal of doing it.

Comment 5

[Egor Starkov]

Attachment: workaround patch v2

Comment 6

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

This is a hack that will break things for anyone using content processes outside of Fennec. I don't want to do this.

Why can't we just fix the problem with the scrollport?

Comment 7

[Oleg Romashin (:romaxa)]

> Why can't we just fix the problem with the scrollport?

I think because we never change scrollPort position in Fennec.
cjones said that it is not critical feature and iPhone browser does the same

Comment 8

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

Then maybe we need to check for that more explicitly.

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to comment #7)
> > Why can't we just fix the problem with the scrollport?
> 
> I think because we never change scrollPort position in Fennec.
> cjones said that it is not critical feature and iPhone browser does the same

That's not at all what I said.  I have no idea how mobilesafari reports scroll offsets; our discussion was wrt rendering of position:fixed elements.  At any rate, the frontend folks plan on moving (back to) an implementation that uses scrollTo/scrollBy instead of shifting the displayport around, not sure how much if at all that fixes the issues here.

Comment 10

[Doug Turner (:dougt)]

this is not a blocking bug.  Text selection in content in Fennec is not supported.

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

I think bug 652168 is the way forward