Touchpad gestures don't work anymore with Elantech drivers

VERIFIED FIXED in mozilla2.0b12

Status

()

defect
VERIFIED FIXED
9 years ago
3 years ago

People

(Reporter: stephan, Assigned: Ehsan)

Tracking

(Depends on 1 bug, {regression})

Trunk
mozilla2.0b12
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(Whiteboard: [hardblocker](?)[widgetremoval])

Attachments

(7 attachments, 12 obsolete attachments)

96.66 KB, image/jpeg
Details
1.48 MB, application/x-rar-compressed
Details
19.66 KB, text/plain
Details
2.41 KB, patch
jimm
: review+
Details | Diff | Splinter Review
8.39 KB, patch
jimm
: review+
Details | Diff | Splinter Review
6.72 KB, patch
jimm
: review+
Details | Diff | Splinter Review
14.40 KB, patch
jimm
: review+
Details | Diff | Splinter Review
(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b6pre) Gecko/20100903 Firefox/4.0b6pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0b5pre) Gecko/20100830 Firefox/4.0b5pre

On my laptops touchpad there are two gestures for going back and forward in page navigation (three fingers left or right). This doesn't work anymore. Instead of that it does now page up or page down.

Reproducible: Always

Steps to Reproduce:
1. get a build from latest trunk
2. try to use back or forward gesture on your Laptops Touchpad
Actual Results:  
it does page up or page down

Expected Results:  
it does page back or page forward

Happens since:

Built from http://hg.mozilla.org/mozilla-central/rev/3f499de2401d
Mozilla/5.0 (Windows NT 6.1; rv:2.0b5pre) Gecko/20100830 Firefox/4.0b5pre
(Reporter)

Updated

9 years ago
Version: unspecified → Trunk
(Reporter)

Comment 1

9 years ago
Seems not to happen on Synaptics Touchpads
Happens on Elan Smartpad Touchpads

They seem to use different ways to do the back/forward navigation.
(Reporter)

Updated

9 years ago
Blocks: 589529

Comment 2

9 years ago
Try updating your build because I believe this was bug 589529 which is now fixed.

Comment 3

9 years ago
Same happens here, Elan Touchpad. Kurt I am using the latest hourly from today and it is still broken.
(Reporter)

Comment 4

9 years ago
Still happens with latest nightly.

Comment 5

9 years ago
Also happens on the Beta 5 candidate, any chance of a quick fix before release?
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 6

9 years ago
This is break before or after bug 589529 was fixed?  Or is this also a regression from bug 130078
blocking2.0: --- → ?
(Reporter)

Comment 7

9 years ago
I think it happened before 589523 was fixed.
Not sure about 130078. Should I add 130078 as block?

It happens exactly since this hourly build: http://hg.mozilla.org/mozilla-central/rev/3f499de2401d

Comment 8

9 years ago
(In reply to comment #7)
> I think it happened before 589523 was fixed.
> Not sure about 130078. Should I add 130078 as block?
> 
> It happens exactly since this hourly build:
> http://hg.mozilla.org/mozilla-central/rev/3f499de2401d

Only if you can confirm that it was caused by that bug or most likely caused by it.  Can you try the 08-27 and 08-28 nightlys and see if it worked in the 27ths build and broken in the 28ths?

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2010-08-27-04-mozilla-central/
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2010-08-28-04-mozilla-central/
(Reporter)

Comment 9

9 years ago
Works on 27th and 28th nightly.

It started with an hourly from 30th
This is the build ID from "about:support": 20100830165706
This is what "about:buildconfig" says: Built from http://hg.mozilla.org/mozilla-central/rev/3f499de2401d

Comment 10

9 years ago
khuey, looks like bug 589523 caused problems with other trackpads that were originally working after bug 130078 landed.
Hopefully Bug 591874 will fix this.
Assignee: nobody → khuey
Component: General → Widget: Win32
Product: Firefox → Core
QA Contact: general → win32
This might not be fixable, but I guess we should block until it becomes clear it isn't.
blocking2.0: ? → final+

Updated

9 years ago
Depends on: 591874
(Reporter)

Comment 13

9 years ago
Latest hourly which includes fixed Bug 591874, doesn't fix this Bug. It still does page up/down instead of back/forward.

There are lots of people out there with an Elan Smartpad Touchpad. So hopefully this gets fixed, as this is something which makes people change their browser.

Comment 14

9 years ago
Confirmed with Morpog, swipe back and forward is still broken with the patch checked in. I am a very upset user, I am willing and able to try test builds in order to get this resolved.
I doubt there's much we can do here.  The only thing I can think of is to try enabling the a11y emulation if we detect one of these drivers.
(Reporter)

Comment 16

9 years ago
Maybe some of you developers could try to get in contact with Elantech.
http://www.emc.com.tw/eng/about_elan_5.asp

Comment 17

9 years ago
Kyle, would you be able to put up a tryserver build with this emulation so I can test it? So long as it works for now, at least it is better than nothing. However a real fix would be a much better solution.
(Reporter)

Comment 18

9 years ago
I'm also interested into a tryserver build, so we can try nad confirm if an emulation does the trick.
Alex, is there an easy way to force the HWND emulation that you added to be on?
(In reply to comment #19)
> Alex, is there an easy way to force the HWND emulation that you added to be on?

HWND emulation is a part of accessibility and we enable it if we detect the presence of certain screen readers. It looks like accessibility isn't wanted to make mouse gestures work. If HWND emulation is right way to fix this bug then it should be moved out from accessibility, so the answer is rather no than yes.
Is there something easy we can do to test if the emulation fixes this bug before investing more time into moving the emulation out of accessibility?
Say, flipping a pref, or a small patch we can apply that forces the emulation on for a try server build?
yes, you should make nsWinUtils::IsWindowEmulationEnabled to return true (http://mxr.mozilla.org/mozilla-central/source/accessible/src/msaa/nsWinUtils.cpp#135) always and then start accessibility: you could run any screen reader (like NVDA nvda-project.org), AT tool (like Accessible Event Watcher) or you could use DOM inspector (by inspecting the accessible tree).
(In reply to comment #22)
> Say, flipping a pref, or a small patch we can apply that forces the emulation
> on for a try server build?

no perf. If you mean remote testing of try server build by running mochitests then you should morph tests to start a11y, that can be done by getting nsIAccessbilityService service, btw, you could do the same locally instead of approaches described above.
Could someone who sees this bug try this try server build
http://stage.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/tnikkel@gmail.com-de9af3868036/
and before testing you need to install DOM Inspector (https://addons.mozilla.org/en-US/firefox/addon/6622/) then open it from the Tools menu, then click on the dropdown icon to the left of "Document - DOM Nodes" near the top left of the window and choose Accessible Tree. Then see if the touchpad things work or not.
(Reporter)

Comment 26

9 years ago
Just tried it. Doesn't help. Still does scrolling.

Comment 27

9 years ago
Timothy, I tried that, but the scrolling is not working. Keep the tryservers coming, I am willing and able.
Wait, is this bug about back/forward gestures not working or scrolling gestures not working?
(Reporter)

Comment 29

9 years ago
back/forward gestures not working.

Comment 30

9 years ago
Yup, swiping left and right is not working ( three finger )
So then comment 27 was mistaken when it talked about scrolling not working?

Comment 32

9 years ago
Yep it was mistaken, I realised after you replied, but there is no way to remove posts? Sorry for the miscommunication!
Kyle, would we need to hook up event handling or something to the emulated HWNDs or would you expect this to just work if the HWNDs are present?
If I had to guess I'd think that we need to forward messages from these emulated HWNDs back up to the real HWND to be dispatched appropriately.

It's possible that the drivers rely on focus/size of the various HWNDs though, in which case this is probably not fixable on our side.

Comment 35

9 years ago
Guys, I noticed that pinch and zoom is also broken. Should I make a seperate bug, or can this one be modified to reflect this issue?
Probably the same issue.

Comment 37

9 years ago
I hate to bugspam, but I am willing to test new builds and this bug seems to have been forgotten about?
This is probably different from bug 594977 since it doesn't involve Synaptics hardware/drivers, as I understand it.

Can someone with this Elan Touchpad identify what drivers it uses? Is there a special page in the Mouse Control Panel? Is there any touchpad helper application running like SynTPEnh.exe for Synaptics Touchpads?
(Reporter)

Comment 39

9 years ago
Posted image driver screenshot
- It uses the (latest) driver shown in the attached screenshot
- there is a special page in the Mouse control panel
- there seems to be a touchpad helper application (etdctrl.exe)
Great thanks! Too bad I don't read German :-). Under \Program Files\Elantech, are there any text files that mention Mozilla or Firefox anywhere?
(Reporter)

Comment 41

9 years ago
Posted file Elantech directory
Whole Elantech directory from "Program Files" as Winrar package
Thanks!

ETDApix.dll contains one occurrence of MozillaWindowClass and multiple occurrences of MozillaUIWindowClass. ETDCmds.dll contains one occurrence of MozillaUIWindowClass. So it's a safe bet that Elantech is sniffing for MozillaUIWindowClass and not finding it, thus breaking something.
Maybe we could detect Elantech and switch our toplevel window class to MozillaUIWindowClass?

Morpog, can you find any registry entries for Elantech's drivers?
(Reporter)

Comment 44

9 years ago
Yes there are alot settings in registry. Check the INF file I uploaded.
Attachment #483594 - Attachment mime type: application/octet-stream → text/plain
OK, I suggest adding another hack to detect Elantech in the registry and if so switch the window class name to MozillaUIWindowClass.

Comment 46

9 years ago
Robert, Elantech have released updated drivers 8.0.7.0 that fix swiping however zooming is broken. The driver is however unnofficial in respect of my laptop. So not sure if Elan are aware, but it seem they have changed their detection? 

Here is the link to the driver, if you want to me to post anything let me know.

http://www.station-drivers.com/page/elantech.htm
Summary: Back and Forward gesture on Touchpad don't work anymore → Touchpad gestures don't work anymore with Elantech drivers
(Reporter)

Comment 47

9 years ago
Unfortunately those 8.0.7.0 drivers don't work with my Elantech Trackpad at all. Maybe they only work with new touchpad models from Elantech. I also tried those unofficial 7.0.7.0 drivers (i had last official 7.0.6.2), but those didn't fix it.

That 8.0.7.0 INF File shows alot new Registry settings. Some are related to Firefox.
I'm probably not going to be able to fix this without access to hardware.
Assignee: khuey → nobody
We can purchase hardware to test if needed.  Can someone drop me an email indicating the exact hardware/software we need to acquire, please?
Anybody seeing this bug care to chime in with what hardware they have?  Google isn't telling me much about what machines have Elantech touchpads, other than that they seem to be common among Asus netbooks.
(Reporter)

Comment 51

9 years ago
I got an EeePC 1000H. Should be cheap to get used on ebay.

Comment 52

9 years ago
I have an Asus N61Jq, with elantech touchpad.

Comment 53

9 years ago
Nearly 2 months and nobody interested in this bug?
We just haven't had time to work on it yet. We will soon.
Whiteboard: [hardblocker](?)
Whiteboard: [hardblocker](?) → [hardblocker](?)[widgetremoval]
I got my hands on an Eee PC 1000, and can reproduce the problem.  The three finger swipe gesture now seems to generate page up/down events.
Status: NEW → ASSIGNED
This patch adds a hack that makes the three finger swipe gesture work again
with v7 of the Elantech drivers.  With the HWND change, the driver now
dispatches Page Up and Page Down keypress messages rather than Alt+Left
and Alt+Right.  It does so in a way that can be differentiated from regular
Page Up/Down presses though -- it sends a virtual key 0xFF WM_KEYDOWN
message just before the VK_PRIOR/VK_NEXT WM_KEYDOWN/WM_KEYUP pair, and
then a 0xFF WM_KEYUP afterwards.  (0xFF is sometimes used for the Fn key
on laptop keyboards.)  The patch detects this 0xFF + Page Up/Down keypress
and translates it into an Alt+Left/Right before passing it off to the
event dispatch code.

The patch also factors out some of the Trackpoint hack code to reuse it
for this hack.
Attachment #505285 - Flags: review?(jmathies)
I can reproduce the problem mentioned with the pinch-to-zoom gesture on v8 of the drivers, but haven't diagnosed it yet.  I can actually trigger it, but it's harder than with v7 -- I find I need to do larger and quicker pinches for the zoom to happen.  The zooming is achieved by the driver sending Alt+mousewheel messages.
(Reporter)

Comment 58

8 years ago
Is there a tryserver build for that patch?
(Reporter)

Comment 60

8 years ago
That build doesn`t work for me. A side by side error pops up after starting firefox.exe
Cameron: you'll need to make an opt build if you want anybody who doesn't have MSVC 2005 installed to test it.

Comment 62

8 years ago
Cameron, I am eager to test but as mentioned I get a side by side error.
(Reporter)

Comment 64

8 years ago
Thanks for providing that tryserver build.

Three finger swipe gesture works great now with v7 drivers on my EeePC 1000H!

Pinch to zoom doesn't work well at all with v7 drivers. It's very choppy and you need to pinch alot to get it start to zoom at all.

Comment 65

8 years ago
Three finger not working here. Using the 7.0.5.10 drivers?
Comment on attachment 505285 [details] [diff] [review]
Part 1: Add a hack for the Elantech touchpad so that three-finger swipe performs a browser back/forward action instead of page up/down.

I have 7.0.6.0, but now am finding it's back to page up/down behaviour.  Will investigate.
Attachment #505285 - Flags: review?(jmathies)
Recall/Morpog: can you try this build?

http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/cmccormack@mozilla.com-b64858cb29a1/try-w32/firefox-4.0b10pre.en-US.win32.zip

It seems some versions of the driver use virtual key 0xFF (Fn) has the modifier when sending the Page Up/Down keys, while others use 0xCC (an OEM defined key).
(Reporter)

Comment 68

8 years ago
Three finger swipe gesture still works great now with 7.0.7.0 drivers on my EeePC 1000H.

Pinch to zoom still doesn't work well. It's very choppy and
you need to pinch alot to get it start to zoom at all.

Comment 69

8 years ago
Hi Cameron. I just tested with 8.0.5.0 drivers and it works fine with them, zooming is good and also back forward and up down work fine. However with the officially latest drivers for my laptop which are 7.0.5.10 back forward works fine, so does up and down. However pinch and zoom fails to respond.
Great.  (I haven't actually made any changes to fix the zooming yet.)
Attachment #505285 - Attachment is obsolete: true
Attachment #506505 - Attachment is obsolete: true
Attachment #506505 - Flags: review?(jmathies)
Comment on attachment 506507 [details] [diff] [review]
Add a hack for the Elantech touchpad so that three-finger swipe left/right performs a browser back/forward action instead of page up/down (v2.1)

Remove a line that was part of the unfinished zoom gesture fix part.
Attachment #506507 - Flags: review?(jmathies)
Here is a build to try that should make the zoom gesture work:

http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/cmccormack@mozilla.com-18cc4ddb8e9c/try-w32/firefox-4.0b10pre.en-US.win32.zip

Can you verify that this works for you Morpog/Recall?  The zooming is somewhat dysfunctional, in that sometimes the driver helper application sends some incorrectly ordered messages -- instead of

  WM_KEYDOWN VK_CONTROL
  WM_MOUSEWHEEL
  WM_KEYUP VK_CONTROL

it sends

  WM_KEYDOWN VK_CONTROL
  WM_KEYUP VK_CONTROL
  WM_MOUSEWHEEL

the result of which is that sometimes when doing a pinch-to-zoom gesture, you will get some page scrolling too.  It does this even for Firefox 3.6, AFAICT, so with this build the regression is resolved, but is not entirely satisfactory behaviour.

(I think it should be possible to detect that these misordered messages are sent from the driver and to assume they are Ctrl+Wheel messages, so I will look at that next.)
(Reporter)

Comment 75

8 years ago
Works alot better than before, but not as good/smooth as in other applications (like windows image viewer). Sometimes it scrools while zooming as you said.
Turns out accounting for the misordered messages isn't an easy fix.  Sometimes the helper application sends more WM_KEYDOWN/WM_KEYUP pairs than WM_MOUSEWHEELs.  When this happens, the WM_MOUSEWHEEL isn't sent while the 1x1 px touchpad helper window is under the mouse cursor, so it's not possible to distinguish it from a normal mouse wheel message from scrolling.

In fact, it looking at the timestamps of the messages when WM_MOUSEWHEEL is sent, our window procedure gets them like this (when the problem surfaces):

  WM_KEYDOWN VK_CONTROL    time t0
  WM_KEYUP VK_CONTROL      time t2
  WM_MOUSEWHEEL            time t1

so I don't know what's happening there.  (I don't know enough about Windows message queues to know if that is reasonable.)

The touchpad helper program seem to have this problem with, say, the Windows Image Viewer program, but for now I don't know why.  I will trace through the helper program to see if there's an peculiarities about how it delivers us the messages.
It sounds like we could be reaching the point of diminishing returns. It sounds to me like maybe we should go with the fix in comment #74 and not block on any more work.
This patch searches for the window that would be underneath the 1x1 px window that the touchpad helper application places under the cursor when doing the pinch-to-zoom gesture, and ensures that the Ctrl+Wheel messages are not discarded.  (The existing code looks to see what window is under the cursor to determine where to dispatch the mouse wheel event.)

IsOurProcessWindow and FindOurProcessWindow are also made static in the class declaration, which masayuki confirmed they should be.
Attachment #507370 - Flags: review?(jmathies)
(Reporter)

Comment 81

8 years ago
This works very very well! I don't think it can get any better on that old and slow ATOM CPU.

Nice done!
The misordering of the key/wheel messages actually was due to the way the message loop peeked the queue for UI events.  Currently, all key messages are retrieved from the message queue and dispatched before all mouse messages are.  This patch makes the message loop retrieve key and mouse messages in order from the queue.
Attachment #508144 - Flags: review?(jmathies)
As mentioned, with the particular v8 driver I've tested with, the WM_KEYDOWN/WM_KEYUP messages sent as part of the zoom gesture are sent with erroneous timestamps -- always a constant 10.  They thus always get retrieved first from the message queue.  This patch detects these messages and sets a flag to assume that all WM_MOUSEWHEEL messages sent before the current system time should be dispatched as events with the control key down.

(It also renames the pref from ui.elantech_gesture_hack.enabled to ui.elantech_gesture_hacks.enabled since there are really two separate hacks that it enables -- one for the three-finger swiping, one for the pinch-to-zoom.)
Attachment #508149 - Flags: review?(jmathies)
Attachment #508149 - Attachment description: Work around the Elantech v8 driver's erroneous message time stamps for zoom gestures → Part 4: Work around the Elantech v8 driver's erroneous message time stamps for zoom gestures

Comment 84

8 years ago
Hi Cameron, sorry for not replying been busy with home life and work. Anyway just tested pinch zoom and back and forward on the 7.0.5.10 which are latest official for my notebook and I pinch zoom works even better than before ( smooth rather than jerky ) Also back forward also works. I think Elantech really need to sort out their driver team, as there are way too many variations in their drivers.
Excellent, thanks very much for testing.

Updated

8 years ago
Whiteboard: [hardblocker](?)[widgetremoval] → [hardblocker](?)[widgetremoval][hsa patch]
Whiteboard: [hardblocker](?)[widgetremoval][hsa patch] → [hardblocker](?)[widgetremoval][has patch]
Comment on attachment 506507 [details] [diff] [review]
Add a hack for the Elantech touchpad so that three-finger swipe left/right performs a browser back/forward action instead of page up/down (v2.1)

mostly nits - 

>+  // Elantech Touchpad Gesture Hack (Bug 593372)

No need for this, hg blame will track changes.

>+PRBool nsWindow::UseInputHack(const char* aPrefName, PRBool aDefaultValue)

Lets get rid of PRBool aDefaultValue and just return the flag value. Also, lets change 'UseInputHack' to something a bit more friendly, like GetInputPref or something similar.

> {
>   nsresult rv;
>   nsCOMPtr<nsIPrefBranch> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
>   if (NS_SUCCEEDED(rv) && prefs) {
>     PRInt32 lHackValue;
>-    rv = prefs->GetIntPref("ui.trackpoint_hack.enabled", &lHackValue);
>+    rv = prefs->GetIntPref(aPrefName, &lHackValue);


Add a null check on aPrefName.


> static PRBool
>-IsObsoleteSynapticsDriver()
>+GetRegistryKey(HKEY aRoot, LPCWSTR aKeyName, LPCWSTR aValueName, PRUnichar* aBuffer, DWORD aBufferLength)
> {

Null checks on passed in params before we go digging in the registry. You could also have this just return a length if aBuffer is null. Add a comment above this describing the params as well.

Also, LPCWSTR and PRUnichar should be interchangeable, so try using PRUnichar for everything.
Attachment #506507 - Flags: review?(jmathies) → review-
Comment on attachment 507370 [details] [diff] [review]
Part 2: Ensure that Elantech driver helper window doesn't prevent zoom gestures from working

>+EXTRA_DSO_LDOPTS += psapi.lib
>+

>+  if (hProcess) {
>+    PRUnichar path[256];
>+    if (::GetProcessImageFileName(hProcess, path, sizeof path)) {
>+      path[255] = 0;

According to msdn, this is only available on XP and up. We still support builds on Win2K so it looks like you're going to have to load this manually using GetProcAddress.

Updated

8 years ago
Attachment #507370 - Flags: review?(jmathies) → review-
Comment on attachment 508144 [details] [diff] [review]
Part 3: Always process mouse and keyboard events in the right order

Nice cleanup. This definitely needs a try run before landing.
Attachment #508144 - Flags: review?(jmathies) → review+
Comment on attachment 508149 [details] [diff] [review]
Part 4: Work around the Elantech v8 driver's erroneous message time stamps for zoom gestures

Looks to be a slight modification of the first patch, could you merge this together when you resubmit? (Also, please nix all the (Bug 593372) references in this one.
(In reply to comment #86)
> >+  // Elantech Touchpad Gesture Hack (Bug 593372)
> 
> No need for this, hg blame will track changes.

OK.

> >+PRBool nsWindow::UseInputHack(const char* aPrefName, PRBool aDefaultValue)
> 
> Lets get rid of PRBool aDefaultValue and just return the flag value.

Where would the default value come in to play, if it is not passed in to this method?  (UseInputHack would have to know about particular pref names, to know what their defaults are.  Passing in the default value seems simplest.)

> Also, lets
> change 'UseInputHack' to something a bit more friendly, like GetInputPref or
> something similar.

All right (though they are quite hacky!).

> > {
> >   nsresult rv;
> >   nsCOMPtr<nsIPrefBranch> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
> >   if (NS_SUCCEEDED(rv) && prefs) {
> >     PRInt32 lHackValue;
> >-    rv = prefs->GetIntPref("ui.trackpoint_hack.enabled", &lHackValue);
> >+    rv = prefs->GetIntPref(aPrefName, &lHackValue);
> 
> 
> Add a null check on aPrefName.

OK.

> > static PRBool
> >-IsObsoleteSynapticsDriver()
> >+GetRegistryKey(HKEY aRoot, LPCWSTR aKeyName, LPCWSTR aValueName, PRUnichar* aBuffer, DWORD aBufferLength)
> > {
> 
> Null checks on passed in params before we go digging in the registry.

OK.

> You could
> also have this just return a length if aBuffer is null.

I will make it survive null being passed in for aBuffer, and still return true/false depending on whether the registry value exists. Returning the length would either mean returning a special value like -1 to indicate non-existence, or passing in a PRUint32* in which the length would be stored.  We don't need that information yet, so I don't think we need to return it at the moment.

> Add a comment above
> this describing the params as well.

OK.

> > static PRBool
> >-IsObsoleteSynapticsDriver()
> >+GetRegistryKey(HKEY aRoot, LPCWSTR aKeyName, LPCWSTR aValueName, PRUnichar* aBuffer, DWORD aBufferLength)
> > {
> 
> 
> Also, LPCWSTR and PRUnichar should be interchangeable, so try using PRUnichar
> for everything.

OK.

(In reply to comment #87)
> We still support builds
> on Win2K so it looks like you're going to have to load this manually using
> GetProcAddress.

How about those WINCE ifdefed parts?  I didn't worry about them at all.  Is it OK to ignore them?

(In reply to comment #88)
> Nice cleanup. This definitely needs a try run before landing.

I got a successful try run earlier, but I'll run another one after addressing your comments.

(In reply to comment #89)
> Looks to be a slight modification of the first patch, could you merge this
> together when you resubmit?

The rename of the pref/member variable/functions?  OK.

> (Also, please nix all the (Bug 593372) references
> in this one.

OK.
> > >+PRBool nsWindow::UseInputHack(const char* aPrefName, PRBool aDefaultValue)
> > 
> > Lets get rid of PRBool aDefaultValue and just return the flag value.
> 
> Where would the default value come in to play, if it is not passed in to this
> method?  (UseInputHack would have to know about particular pref names, to know
> what their defaults are.  Passing in the default value seems simplest.)
> 

Maybe I missed something, I didn't see where aDefaultValue was being used. I thought what you needed here was a boolean result based on the pref value (true if it's set and true, false if it's false or not set). I'll wait for your next rev to take a look.


> (In reply to comment #87)
> > We still support builds
> > on Win2K so it looks like you're going to have to load this manually using
> > GetProcAddress.
> 
> How about those WINCE ifdefed parts?  I didn't worry about them at all.  Is it
> OK to ignore them?

Ignore them. We don't actively support ce anymore.

Updated

8 years ago
Attachment #508149 - Flags: review?(jmathies)
(In reply to comment #91)
> Maybe I missed something, I didn't see where aDefaultValue was being used. I
> thought what you needed here was a boolean result based on the pref value (true
> if it's set and true, false if it's false or not set). I'll wait for your next
> rev to take a look.

The result of IsObsoleteElantechDriver() is passed in as the aDefaultValue.  aDefaultValue is the true/false value to use when the pref is set to autodetect whether or not to use the hack (-1, or lack of the pref, means autodetect).

(I seem to have uploaded the wrong patches, please wait.)
Comment on attachment 510502 [details] [diff] [review]
Part 1: Add a hack for the Elantech touchpad so that three-finger swipe left/right performs a browser back/forward action instead of page up/down (v3)

Pushed the wrong patches to tryserver, too.  Will attach new patches after another try run.
Attachment #510502 - Attachment is obsolete: true
Attachment #510502 - Flags: review?(jmathies)
Attachment #510503 - Attachment is obsolete: true
Attachment #510503 - Flags: review?(jmathies)
Attachment #510504 - Attachment is obsolete: true
Attachment #510504 - Flags: review?(jmathies)
Addressed review comments.

* I used InitInputWorkaroundPrefDefaults as the friendlier replacement name for
  InitInputHackDefaults.  Also s/UseInputHack/GetInputWorkaroundPref/.
* Renamed the GetInputWorkaroundPref's aDefault argument to aValueIfAutomatic to
  make it clearer, hopefully.  Also added some documentation for that function.
Attachment #510545 - Flags: review?(jmathies)

Comment 100

8 years ago
Cameron, before you go away can you link me to a tryserver build. I will test it for you and post my feedback here.
Whiteboard: [hardblocker](?)[widgetremoval][has patch] → [hardblocker](?)[widgetremoval][has patch][needs review]

Comment 102

8 years ago
(In reply to comment #101)
> I think this is the try build you want
> http://stage.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/cmccormack@mozilla.com-aec6985388a0/

Thank Timothy! I can now confirm it works well with both my native drivers 7.0.5.10 and the latest from Asus which are 8.0.5.0.
blocking2.0: final+ → betaN+

Updated

8 years ago
Attachment #510545 - Flags: review?(jmathies) → review+

Updated

8 years ago
Attachment #510546 - Flags: review?(jmathies) → review+

Updated

8 years ago
Attachment #510547 - Flags: review?(jmathies) → review+

Updated

8 years ago
Keywords: checkin-needed
Whiteboard: [hardblocker](?)[widgetremoval][has patch][needs review] → [hardblocker](?)[widgetremoval]
(Assignee)

Comment 103

8 years ago
http://hg.mozilla.org/mozilla-central/rev/87fb25283d8d
http://hg.mozilla.org/mozilla-central/rev/c0ee311faca1
http://hg.mozilla.org/mozilla-central/rev/0da0c99b690d
http://hg.mozilla.org/mozilla-central/rev/d2db6c6be747

My apologies for the checkin author on the last two patches.  They didn't have a user name embedded, and I assumed that they did without checking based on the first two patches.  :(
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
This broke scrolling on Alps touchpads (at least on my HP dv4t laptop w/ Win7). Firefox now zooms when I attempt to scroll using the touchpad. FWIW. The scroll gesture was working fine for me up to this point.
I was pointed to the ui.elantech_gesture_hacks.enabled pref on IRC. Setting it to 0 and restarting fixed my problem.
Ehsan, not sure why my hg exports didn't get the name in there, sorry about that.

Ryan, it seems suboptimal for this patch to break scrolling on Alps touchpads with default pref settings.  Maybe the autodetection logic needs to be tweaked.  Can you file a followup bug?  I will look into it on Monday.  Thanks.
Depends on: 632654
http://hg.mozilla.org/mozilla-central/rev/8ba0d8f0efb2

backed out, this broke scroll wheel functionality on windows.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
There's something very very wrong with mAssumeWheelIsZoomUntil.
This also blew up thunderbird builds:

e:\buildbot\win32-comm-central-build\build\mozilla\widget\src\windows\nsWindow.h(384) : warning C4244: 'return' : conversion from 'LPARAM' to 'WORD', possible loss of data

e:/buildbot/win32-comm-central-build/build/mozilla/widget/src/windows/nsWindow.cpp(6732) : warning C4018: '<' : signed/unsigned mismatch

e:/buildbot/win32-comm-central-build/build/mozilla/widget/src/windows/nsWindow.cpp(7739) : error C2664: 'LoadLibraryA' : cannot convert parameter 1 from 'const unsigned short [10]' to 'LPCSTR'

        Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast

e:/buildbot/win32-comm-central-build/build/mozilla/widget/src/windows/nsWindow.cpp(7758) : error C2664: 'DWORD (HANDLE,LPTSTR,DWORD)' : cannot convert parameter 2 from 'PRUnichar [256]' to 'LPTSTR'

        Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast

e:/buildbot/win32-comm-central-build/build/mozilla/widget/src/windows/nsWindow.cpp(7760) : error C2664: 'lstrlenA' : cannot convert parameter 1 from 'PRUnichar [256]' to 'LPCSTR'

        Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast

e:/buildbot/win32-comm-central-build/build/mozilla/widget/src/windows/nsWindow.cpp(7762) : error C2664: 'lstrcmpiA' : cannot convert parameter 1 from 'PRUnichar *' to 'LPCSTR'

        Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast

Updated

8 years ago
Depends on: 632707
Looks like mAssumeWheelIsZoomUntil isn't initialized.

Not sure about the Thunderbird build problems.  Do they compile without -DUNICODE?
(In reply to comment #110)
> Looks like mAssumeWheelIsZoomUntil isn't initialized.
> 
> Not sure about the Thunderbird build problems.  Do they compile without
> -DUNICODE?

Yes, we need to explicitly request the ascii or wide character apis in widget. So the str apis need to be specific, and calls like LoadLibrary needs to be LoadLibraryW/A.
OK.  (I had a feeling it was OK to assume -DUNICODE for some reason, but I guess I was wrong.)

Another issue hidden in those warnings is that mAssumeWheelIsZoomUntil is unsigned, while the result of ::GetMessageTime() is signed.  It sounds like ::GetMessageTime() values wrap back around to zero once they reach 2**31-1, while ::GetTickCount() values wrap at 2**32-1.  So that timestamp wrapping checking code at the top of nsWindow::ProcessMessage probably needs checking.

(http://tbpl.mozilla.org/?tree=MozillaTry&pusher=cmccormack@mozilla.com is building with the mAssumeWheelIsZoomUntil intialization.)

/me enplanes
Comment on attachment 508144 [details] [diff] [review]
Part 3: Always process mouse and keyboard events in the right order

This always used to bug me (particularly on W2K where key and mouse events don't get thrown away after 2 seconds) - I'm used to waiting for Gecko to respond to a click so that I can be sure that it's safe to type!
Comment on attachment 510546 [details] [diff] [review]
Part 2: Ensure that Elantech driver helper window doesn't prevent z oom gestures from working (v2.1)

>+static HWND FindTopmostWindowAtPoint(HWND aHWND, const POINT& aPoint)
(Why doesn't this use [Child]WindowFromPoint[Ex]?)

Non-Unicode fixes required:
>+typedef DWORD (*GetProcessImageFileNameProc)(HANDLE, LPTSTR, DWORD);
LPWSTR

>+  static HMODULE hPSAPI = ::LoadLibrary(L"psapi.dll");
LoadLibraryW

>+    PRUnichar path[256];
WCHAR (for mingw, but also in case we turn off -Zc:wchar_t-)

>+      int pathLength = lstrlen(path);
lstrlenW

>+        if (lstrcmpi(path + pathLength - filenameSuffixLength, filenameSuffix) == 0) {
lstrcmpiW

Updated

8 years ago
Whiteboard: [hardblocker](?)[widgetremoval] → [hardblocker](?)[widgetremoval][has patch]
(Assignee)

Updated

8 years ago
Assignee: cam → ehsan
Whiteboard: [hardblocker](?)[widgetremoval][has patch] → [hardblocker](?)[widgetremoval]
(Assignee)

Comment 115

8 years ago
I wasn't able to reproduce the scrolling break-age on a local build.  Does anybody have any idea how to reproduce it?
(In reply to comment #115)
> I wasn't able to reproduce the scrolling break-age on a local build.  Does
> anybody have any idea how to reproduce it?

On a system that doesn't have an elantech touchpad the scroll wheel changes zoom with the patches applied.
(Assignee)

Comment 117

8 years ago
Make the code Unicode-correct.
Attachment #510546 - Attachment is obsolete: true
Attachment #511608 - Flags: review?(jmathies)
(Assignee)

Comment 118

8 years ago
With mAssumeWheelIsZoomUntil initialized and the rounding check corrected (which was incorrect for reasons beside the signed-ness; i.e., it was using 0x8000000 instead of 0x80000000).
Attachment #510547 - Attachment is obsolete: true
(Assignee)

Comment 119

8 years ago
I wasn't able to reproduce the original scrolling problem in the end.  I've fixed a bunch of problems with these patches and have sent them to the try server.  The builds should be available in a couple of hours here:

<http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/eakhgari@mozilla.com-4f2be7d88eac>

It would be great if people can test those builds to see if the mouse scroll wheel problem has been fixed.

CCing some QA folks too.
Status: REOPENED → ASSIGNED
Keywords: qawanted
Whiteboard: [hardblocker](?)[widgetremoval] → [hardblocker](?)[widgetremoval][has patch][needs testing]
(Assignee)

Comment 120

8 years ago
This patch incorporates some other things that Cameron has fixed in his try push today <http://hg.mozilla.org/try/rev/eb5376d1b3cd>.
Attachment #511611 - Attachment is obsolete: true
Attachment #511627 - Flags: review?(jmathies)
(Assignee)

Comment 121

8 years ago
I also pushed another try server build with the original patches landed as part of this bug, for people to test for comparison:

<http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/eakhgari@mozilla.com-bcfa6ea99a85>

If the new patches here are effective, the build in this comment should change the zoom on wheel scroll, and the build in comment 119 should not show that behavior.
I think the scrolling when unintended happens just due to mAssumeWheelIsZoomUntil being uninitialized.  If it happened to start off with a high enough number in it, all mouse wheels would be processed as if control was pressed down.

Thanks for driving the fixes to my patches, Ehsan!
(Assignee)

Comment 123

8 years ago
(In reply to comment #119)
> I wasn't able to reproduce the original scrolling problem in the end.  I've
> fixed a bunch of problems with these patches and have sent them to the try
> server.  The builds should be available in a couple of hours here:
> 
> <http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/eakhgari@mozilla.com-4f2be7d88eac>

I had an embarrassing typo in my patch which caused a build failure.  Builds with the fix will hopefully be available here:

<http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/eakhgari@mozilla.com-6860167b3daf>
(Assignee)

Comment 124

8 years ago
(In reply to comment #122)
> I think the scrolling when unintended happens just due to
> mAssumeWheelIsZoomUntil being uninitialized.  If it happened to start off with
> a high enough number in it, all mouse wheels would be processed as if control
> was pressed down.
> 
> Thanks for driving the fixes to my patches, Ehsan!

That's what I suspect too.  I'll do more testing tomorrow, and will also ask other people to test as well.

Comment 125

8 years ago
Ehsan, I tested all the builds you posted. All worked fine without breaking anything on my Elan Touchpad. However your last build <http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/eakhgari@mozilla.com-6860167b3daf> crashes at startup.

Here is the log

AvailableVirtualMemory: 4157898752
BuildID: 20110210220707
CrashTime: 1297443638
InstallTime: 1297443491
ProductName: Firefox
SecondsSinceLastCrash: 133
StartupTime: 1297443638
SystemMemoryUsePercentage: 30
Throttleable: 1
TotalVirtualMemory: 4294836224
URL: 
Vendor: Mozilla
Version: 4.0b12pre

This report also contains technical information about the state of the application when it crashed.
(In reply to comment #125)
> Ehsan, I tested all the builds you posted. All worked fine without breaking
> anything on my Elan Touchpad. However your last build
> <http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/eakhgari@mozilla.com-6860167b3daf>
> crashes at startup.

Hmm, does that happen every time for you?  Can you try and get a stack trace please?  <https://developer.mozilla.org/en/how_to_get_a_stacktrace_with_windbg>

Instead of the ".sympath" line in that page, you would want to use this:

.sympath SRV*c:\symbols*http://build.mozilla.org/tryserver-symbols/;SRV*c:\symbols*http://msdl.microsoft.com/download/symbols

Thanks!
If I would help, I will drive to Fry's and buy some ASUS netbooks for testing.  Let me know.
(Assignee)

Comment 128

8 years ago
So far I've run the build in comment 121 (which should have the problem) and the build in comment 123 (which hopefully doesn't have the problem) about 100 times each.  I have managed to reproduce the scroll wheel bug in the build from comment 121 only once, and never in the build in comment 123.

I don't have any device with an Elantech touchpad...

(In reply to comment #127)
> If I would help, I will drive to Fry's and buy some ASUS netbooks for testing. 
> Let me know.

That may be a good idea.  I asked roc whether we have devices with Elantech touchpads, and he said that he thinks that we do, but he doesn't know where...
(In reply to comment #123)
> (In reply to comment #119)
> > I wasn't able to reproduce the original scrolling problem in the end.  I've
> > fixed a bunch of problems with these patches and have sent them to the try
> > server.  The builds should be available in a couple of hours here:
> > 
> > <http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/eakhgari@mozilla.com-4f2be7d88eac>
> 
> I had an embarrassing typo in my patch which caused a build failure.  Builds
> with the fix will hopefully be available here:
> 
> <http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/eakhgari@mozilla.com-6860167b3daf>

Working as expected here. Scrolling works fine. (As mentioned on irc I can't test the Elantech changes though.)
Comment on attachment 511608 [details] [diff] [review]
Part 2: Ensure that Elantech driver helper window doesn't prevent z oom gestures from working (v2.1)

>+static PRBool PointInWindow(HWND aHWND, const POINT& aPoint)
>+
>+static HWND FindTopmostWindowAtPoint(HWND aHWND, const POINT& aPoint)
>+

Neil made a good point in his review, couldn't we use ChildWindowFromPoint or RealChildWindowFromPoint in place of these?

Updated

8 years ago
Attachment #511627 - Flags: review?(jmathies) → review+

Comment 131

8 years ago
(In reply to comment #126)
> (In reply to comment #125)
> > Ehsan, I tested all the builds you posted. All worked fine without breaking
> > anything on my Elan Touchpad. However your last build
> > <http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/eakhgari@mozilla.com-6860167b3daf>
> > crashes at startup.
> 
> Hmm, does that happen every time for you?  Can you try and get a stack trace
> please?  <https://developer.mozilla.org/en/how_to_get_a_stacktrace_with_windbg>
> 
> Instead of the ".sympath" line in that page, you would want to use this:
> 
> .sympath
> SRV*c:\symbols*http://build.mozilla.org/tryserver-symbols/;SRV*c:\symbols*http://msdl.microsoft.com/download/symbols
> 
> Thanks!

Crashes every time, also it won't download the symbols.

*** ERROR: Symbol file could not be found.  Defaulted to export symbols for ntdll.dll - 
ntdll!CsrSetPriorityClass+0x40:
00000000`76e80fb0 cc              int     3
0:000> .sympath SRV*c:\symbols*SRV*c:\symbols*http://build.mozilla.org/tryserver-symbols/;SRV*c:\symbols*http://msdl.microsoft.com/download/symbols
Symbol search path is: SRV*c:\symbols*SRV*c:\symbols*http://build.mozilla.org/tryserver-symbols/;SRV*c:\symbols*http://msdl.microsoft.com/download/symbols
Expanded Symbol search path is: srv*c:\symbols*srv*c:\symbols*http://build.mozilla.org/tryserver-symbols/;srv*c:\symbols*http://msdl.microsoft.com/download/symbols
(Assignee)

Comment 132

8 years ago
(In reply to comment #130)
> Comment on attachment 511608 [details] [diff] [review]
> Part 2: Ensure that Elantech driver helper window doesn't prevent z oom
> gestures from working (v2.1)
> 
> >+static PRBool PointInWindow(HWND aHWND, const POINT& aPoint)
> >+
> >+static HWND FindTopmostWindowAtPoint(HWND aHWND, const POINT& aPoint)
> >+
> 
> Neil made a good point in his review, couldn't we use ChildWindowFromPoint or
> RealChildWindowFromPoint in place of these?

Maybe we could, but I don't have a device with an Elantech touchpad, so I wanted to change as little possible to avoid the risk of breaking the fix.  We could file a followup to investigate that.
(Assignee)

Comment 133

8 years ago
So, in order to test the effect of mAssumeWheelIsZoomUntil being uninitialized, I instrumented a build to initialize its value to 0x3dcdcdcd, and with that, I get the zoom on mouse wheel bug.  Therefore, I'm fairly sure that the recent version of Part 4 has fixed that issue.
No scrolling issues here with the latest tryserver build and a fresh profile.
Comment on attachment 511608 [details] [diff] [review]
Part 2: Ensure that Elantech driver helper window doesn't prevent z oom gestures from working (v2.1)

r+'ing for now on the assumption we'll file a follow up for more cleanup.
Attachment #511608 - Flags: review?(jmathies) → review+
(Assignee)

Comment 136

8 years ago
Recall: I've submitted another try build for you.  It should be available in an hour or so: <http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/eakhgari@mozilla.com-cc2545084754>

Can you please try it to see if you see the startup crash with this one too?
(Assignee)

Updated

8 years ago
Depends on: 633572
(Assignee)

Comment 137

8 years ago
(In reply to comment #135)
> Comment on attachment 511608 [details] [diff] [review]
> Part 2: Ensure that Elantech driver helper window doesn't prevent z oom
> gestures from working (v2.1)
> 
> r+'ing for now on the assumption we'll file a follow up for more cleanup.

Filed bug 633572 for that.
(Assignee)

Comment 138

8 years ago
I got a device with an Elantech touchpad, and I can reproduce the startup crash in the build in comment 123.  Investigating...

Comment 139

8 years ago
(In reply to comment #136)
> Recall: I've submitted another try build for you.  It should be available in an
> hour or so:
> <http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/eakhgari@mozilla.com-cc2545084754>
> 
> Can you please try it to see if you see the startup crash with this one too?

Ok will do, will post back with results. I notice you have been able to reproduce the error.
(Assignee)

Comment 140

8 years ago
Comment on attachment 510545 [details] [diff] [review]
Part 1: Add a hack for the Elantech touchpad so that three-finger s wipe left/right performs a browser back/forward action instead of page up/down (v2.2)

I think I found the cause of the crash.

>+GetRegistryKey(HKEY aRoot, PRUnichar* aKeyName, PRUnichar* aValueName, PRUnichar* aBuffer, DWORD aBufferLength)

The GetRegistryKey receives PRUnichar* as a pointer to a two byte array.  aBufferLength is in bytes, which means that in this particular case, it's 80 (because |buf|'s size is 40.)

>+{
>+  if (!aKeyName) {
>+    return PR_FALSE;
>+  }
>+
>   HKEY key;
>-  LONG result = ::RegOpenKeyExW(HKEY_LOCAL_MACHINE,
>-      L"Software\\Synaptics\\SynTP\\Install", 0, KEY_READ, &key);
>+  LONG result = ::RegOpenKeyExW(aRoot, aKeyName, NULL, KEY_READ, &key);
>   if (result != ERROR_SUCCESS)
>     return PR_FALSE;
>   DWORD type;
>-  PRUnichar buf[40];
>-  DWORD buflen = sizeof(buf);
>-  result = ::RegQueryValueExW(key, L"DriverVersion", NULL, &type, (BYTE*)buf, &buflen);
>+  result = ::RegQueryValueExW(key, aValueName, NULL, &type, (BYTE*) aBuffer, &aBufferLength);
>   ::RegCloseKey(key);
>   if (result != ERROR_SUCCESS || type != REG_SZ)
>     return PR_FALSE;
>-  buf[NS_ARRAY_LENGTH(buf) - 1] = 0;
>+  if (aBuffer)
>+    aBuffer[aBufferLength - 1] = 0;

Here, we're adding (aBufferLength - 1) to a PRUnichar* pointer.  That is wrong, since aBufferLength is twice the number of items in the array.  So, we end up writing to somewhere down the stack, which happens to be the top two bytes of the nsIWidget* we're trying to get from do_CreateInstance call used to create the first nsWindow object, so the QueryInterface calls ends up crashing.

I'm going to have a fix soon.

Comment 142

8 years ago
Ehsan, did you cancel your build as it says nothing there?

Updated

8 years ago
Attachment #511871 - Flags: review?(jmathies) → review+
(Assignee)

Comment 143

8 years ago
(In reply to comment #142)
> Ehsan, did you cancel your build as it says nothing there?

I've pushed another build which should have fixed the crash problem.  It will be available here: <http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/eakhgari@mozilla.com-71a52bcc3e05>.

It would be great if you can test it.  It will be finished in a few hours I guess.

Thanks!
(Reporter)

Comment 144

8 years ago
Trybuild from Comment 143 works great with Elantech Touchpad (driver 7.0.6.4).
Scrolling and zooming work both very smooth.

No crash at all, tried for an hour.

Can't test 8.x.x.x drivers, as they don't work with my touchpad.

Comment 145

8 years ago
(In reply to comment #143)
> (In reply to comment #142)
> > Ehsan, did you cancel your build as it says nothing there?
> 
> I've pushed another build which should have fixed the crash problem.  It will
> be available here:
> <http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/eakhgari@mozilla.com-71a52bcc3e05>.
> 
> It would be great if you can test it.  It will be finished in a few hours I
> guess.
> 
> Thanks!

Works fine no crashes with either 7.0.5.10 or 8.0.5.0
(Assignee)

Comment 146

8 years ago
Thanks everyone for testing this.  My local tests also show that the gestures are working correctly.
(Assignee)

Updated

8 years ago
Keywords: qawanted
Whiteboard: [hardblocker](?)[widgetremoval][has patch][needs testing] → [hardblocker](?)[widgetremoval][has patch]

Comment 147

8 years ago
Will this make the next beta?
(Assignee)

Comment 148

8 years ago
(In reply to comment #147)
> Will this make the next beta?

Yes, it will.  :-)
(Assignee)

Comment 149

8 years ago
http://hg.mozilla.org/mozilla-central/rev/720d17e28023
http://hg.mozilla.org/mozilla-central/rev/ace573ee5421
http://hg.mozilla.org/mozilla-central/rev/fa7c98cbc339
http://hg.mozilla.org/mozilla-central/rev/e9899b77c6ea
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
Whiteboard: [hardblocker](?)[widgetremoval][has patch] → [hardblocker](?)[widgetremoval]
(Assignee)

Comment 150

8 years ago
It would be really good if you guys could test tomorrow's nightly builds to make sure that this problem is indeed fixed.  Thanks!
(Reporter)

Comment 151

8 years ago
Works great in latest nightly!

Scrolling and Zooming works as it should on my Elantech Touchpad. No crash at all.
Also tried the latest Nightly on my Desktop PC. TThere are also no issues at all with crashes or scrolling with latest nightly.

Thumbs up!
I find no problems with the nightly on the Eee PC I've got here; pinch-to-zoom and three-finger swipe back/forward both work well, and without crashing in the few minutes I've used it for.  This is with both v7 and v8 of the driver.

Comment 154

8 years ago
Works fine here, thanks for all the hard work guys!
Given the amount of positive feedback lets mark this bug as verified fixed.
Status: RESOLVED → VERIFIED

Comment 156

8 years ago
Hello, I'm getting this bug with the Firefox 4.0 that was released today. I have the Elantech 7.0.3.12 drivers, Windows XP, Asus EEE PC 1000HE. The three-finger swipe does PageUp/PageDown, not Back/Forward
junimitsu, if you are able, can you try a more recent driver, say 7.0.5 of later?  I don't think I tried this earlier version of the driver, and if upgrading helps you, I think we can avoid adding further hacks.  If it doesn't, can you please file a new bug for this (and CC me)?

Comment 158

8 years ago
Yep, upgrading to the 7.0.5.16 driver fixed my issue. Thanks!

Comment 159

8 years ago
It looks like the hack used to solve these swipe issues has spawned a new problem with the Page Up/Down keys.
Check the bug report and fix:
https://bugzilla.mozilla.org/show_bug.cgi?id=640587#c48

The issue is being reported to be solved by setting the "ui.elantech_gesture_hacks.enabled" option to 0.
I have the latest ElanTech driver Asus has for my laptop (7.0.5.12) and cannot use the three finger swipes described in this bug report, no matter the value of "elantech_gesture_hacks" nor how many times I restart Firefox, so I am not sure if the hack is still needed

Can someone try to verify if having the hack set to -1 is still required to prevent the three finger gesture issues? If the latest ElanTech drivers fix the problem by themselves, I'd recommend disabling the hack by default.

Updated

8 years ago
Depends on: 640587
You need to log in before you can comment on or make changes to this bug.