Closed Bug 773356 Opened 12 years ago Closed 12 years ago

[browser] Cannot scroll enabled textareas

Categories

(Firefox OS Graveyard :: General, defect)

Product:
Firefox OS Graveyard
Component:
General
Platform:
ARM
All
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-kilimanjaro:+, blocking-basecamp:+)

Status:
RESOLVED FIXED
Project Flags:
blocking-kilimanjaro +
blocking-basecamp +

People

(Reporter: nhirata, Unassigned)

References

Details

Attachments

(1 file)

Patch v1
1.65 KB, patch
roc
: review+
Details | Diff | Splinter Review 
Environment
    Otoro phone, build 2012-07-10
    Gaia commit: d0fc2f0

Repro
1.    launch browser webapp
2.  go to http://people.mozilla.com/~nhirata/html_tp/disabled_textarea%20no%20scroll.html
3.   try to scroll in both the enabled and disabled fields

Expected:
*  you can scroll

Actual:
*   there's no way to scroll

Note: 
1) in fennec native, you can scroll in the enabled text area; there is no scroll indicatros
2) on mac os x desktop firefox, you can scroll in both.

From : https://github.com/mozilla-b2g/gaia/issues/2311#issuecomment-6933190
blocking-basecamp: --- → ?
Component: DOM → General
Product: Core → Boot2Gecko
I reproduced this on my desktop box.  I'm not sure whom to cc here; Chris, do you know?
Dunno, sounds like the panning heuristics not recognizing the input areas.
Felipe says:

<felipe> jlebar: first thing would be to see if this is working: http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsEventStateManager.cpp#1657

so I'll look into this.
On desktop, when I hover my mouse over the textbox and scroll with my scrollwheel, the textbox /is/ scrolled correctly.
There are a number of things that could be happening here, this is very specific to the way things are set up (e.g. what kind of scroll events are being used: wheel, pixel, or line, etc.). My main suggestion would be to ask how they handled this scenario on Fennec.

But here are some things to check, given my non-complete knowledge of how scrolling works:

- The EventStateManager in the content process needs to know that the mouse is over the textbox to scroll it instead of the parent element. It's conceivable that the UI code that handles the scroll gestures would eat the mouse events in the parent if it detects that they represent a scroll instead of a mousemove.

- If the mousemove properly went through, there is the scroll transaction concept that is meant to avoid instantly scrolling a textbox that your pointer might come across in the middle of a page scroll (and keep scrolling the page).
You can set the prefs: 
  mousewheel.transaction.ignoremovedelay
  mousewheel.transaction.timeout
to 0 to check this. More about how it works here: http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsEventStateManager.cpp#2710

- If scroll events are not getting through the process boundary then it could be they don't have the proper type here: http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsEventStateManager.cpp#1657
(iirc pixel scroll has a different eventStructType but I haven't checked)
When I drag my finger across the screen, is that supposed to get sent to the child as a scroll event, or does the child translate the drag into a scroll event?
(In reply to Chris Jones [:cjones] [:warhammer] from comment #2)
> Dunno, sounds like the panning heuristics not recognizing the input areas.

And the magic link is: http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/webapi.js#123

getPannable returns false for the textareas here.  Vivien has the blame on this whole file, so perhaps he has input as to what's the right thing to do here.
That code gets the computed style for the textarea and checks whether any of overflow, overflow-x, and overflow-y is "auto" or "scroll".  In our case, they're all "visible".  And yet it scrolls.
Very bad user experience, pretty broken -> blocking.
Assignee: nobody → justin.lebar+bug
blocking-basecamp: ? → +
blocking-kilimanjaro: --- → +
Attached patch Patch v1 — — Splinter Review 
The change in this patch was at roc's suggestion over IRC.

I've tested this on desktop and it seems to work for the enabled textarea but not a disabled one.  But drag-scrolling is kind of weird on desktop, so I need to test on mobile.  I'm building now, but it'll take a few hours at least, so if someone else is able to test, that'd be much appreciated.
This patch behaves on the device just as it behaves on desktop: You can scroll the textarea, but only after you've focused it.  And you can only focus the enabled textarea.

I guess that's not desired behavior.  I'll poke around further.
> You can scroll the textarea, but only after you've focused it.  And you can only focus the enabled 
> textarea.

So apparently when you click on a disabled textarea, you don't get a mousedown event.  Seems like you only get a mousedown event once the textarea is focused.  :(

I don't know how to work around this, although surely one can...
Olli, could we just fix that?  The blocking of _all_ events on disabled form controls is just nuts.  imo.
Unassigning myself; at this point, it's probably much more efficient if smaug can push this through, so I can focus on code I understand.
Assignee: justin.lebar+bug → nobody
(In reply to Boris Zbarsky (:bz) from comment #13)
> Olli, could we just fix that?  The blocking of _all_ events on disabled form
> controls is just nuts.  imo.

It is. That bug is *old*. Need to figure out what other browser do. IIRC they block some events.
Could we fire chrome-only mousedown events here?
IIRC, one, and perhaps main reason to not fire events has been issues with default handling.
So, need to look at which events are default handled and in which way.
Whether or not the event is chrome only doesn't affect to that.
Oh, hmm, yes it does. But chrome only event would be still a bit odd.
Chrome-only events wouldn't help for textfields in chrome (they are generated by XBL for <datepicker>, <timepicker>, <menulist type="editable"> and the various types of <textbox>).
Smaug, can you own this bug?  If not, who can?
(In reply to Justin Lebar [:jlebar] from comment #20)
> Smaug, can you own this bug?  If not, who can?

Ping on this bug again.  If we can agree on how to do this, I can try to find b2g resources to fix this bug.  But at this point, it's not clear to me what's the right way to fix it.
My guess is that I won't have time for this this week.
You could ask masayuki if he has any spare cycles, or Enn.
How do you think we should approach this, though?  We don't have a clear way forward...
As discussed on IRC, I think B2G should use EventStateManager for scrolling - if just possible - and 
not implement its own stuff in JS.
(In reply to Olli Pettay [:smaug] from comment #23)
> You could ask masayuki if he has any spare cycles, or Enn.

Masayuki, Enn, do either of you have time to figure this out?  We're short on time, bandwidth, and relevant expertise here.
I don't know what events are used for scrolling by B2G.

IIRC, mouse wheel events are not checking if scrollable elements are disabled.

# And I don't like to touch wheel event handling code for now, I'm changing it in bug 719320.
> I don't know what events are used for scrolling by B2G.

mousedown, mouseup, and mousemove.  The relevant code is in dom/browser-element/BrowserElementScrolling.js.

> IIRC, mouse wheel events are not checking if scrollable elements are disabled.

This matches what I've seen: In desktop b2g builds, you can scroll with the mousewheel over a disabled textarea.  But that doesn't work when you try to scroll with your finger.
Hmm, so, the problem is, mousedown/mouseup events are not fired on disabled editor? On either Opera or Google Chrome, mousedown/mouseup events are fired on disabled editors. I think that we should do so too.
> Hmm, so, the problem is, mousedown/mouseup events are not fired on disabled editor?

That is the immediate problem, yes.  Smaug in comment 25 argues that the deeper problem is that we're using JS for scrolling here, and we should rewrite all this logic in C++.  I'm not volunteering for that job.

> On either Opera or Google Chrome, mousedown/mouseup events are fired on disabled editors. I think 
> that we should do so too.

That would be fine with me!
Um, I don't find why mousedown/mouseup events are not fired on content. Where do we stop the event dispatching??

Anyway, the change must be big...
FormControls' PreHandleEvent check disabled status.
Thanks. But I don't have enough time to fix such big bug...
Summary: [browser] Cannot scroll in textfields with scrollbars → [browser] Cannot scroll textfields
Comment on attachment 642028 [details] [diff] [review]
Patch v1

How would you feel about landing this patch as-is?  It fixes enabled textareas, but scrolling disabled textareas still  doesn't work.

This patch is bitrotted; the code now lives in dom/browser-element/BrowserElementScrolling.js.  But it's all the same code afaict.
Attachment #642028 - Attachment description: WIP v1 → Patch v1
Attachment #642028 - Flags: review?(roc)
Summary: [browser] Cannot scroll textfields → [browser] Cannot scroll enabled textareas
No longer blocks: 780174
The can't-scroll-disabled-textareas portion of this bug is now bug 780174.
https://hg.mozilla.org/mozilla-central/rev/c150fce9c39e
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: