Open
Bug 725475
Opened 10 years ago
Updated 9 years ago
some Synaptics drivers direct scrolling to visible plugin regardless of focus or mouse position
Categories
(Core :: Widget: Win32, defect)
Tracking
()
NEW
People
(Reporter: heycam, Unassigned)
Details
Attachments
(2 files, 1 obsolete file)
6.46 KB,
patch
|
jimm
:
review+
|
Details | Diff | Splinter Review |
1.08 KB,
patch
|
Details | Diff | Splinter Review |
Followup for bug 626813. We should try to find a way to convince the Synaptics driver to send scrolling messages to our outer browser window instead of, for example, the scroll bars within the Adobe Reader plugin.
Reporter | ||
Updated•10 years ago
|
Assignee: nobody → cam
Status: NEW → ASSIGNED
Reporter | ||
Comment 1•10 years ago
|
||
Adding this extra hidden scroll bar seems to coax scrolling messages to our window instead of a plugin. (I did not manage to delve deep enough into the trackpad driver code to determine exactly what heuristics are being used to select the plugin.) The only problem is that sometimes when scrolling (especially if I do it quickly) I will run into this assertion: ASSERTION: Received "nonqueued" message 277 during a synchronous IPC message for Window xxxx ("MozillaWindowClass"), sending it to DefWindowProc instead of the normal window procedure. which is ipc/glue/WindowsMessageLoop.cpp:329, message 277 being WM_VSCROLL. Sometimes I also then get a few in a row identifying various different window classes for windows we have (like "MozillaHiddenWindowClass" and "nsAppShell:EventWindowClass") with message 49544 which according to Spy++ is a registered message "PwrMgrBkGndUpdateBatteryRemainingTime". I'm guessing that's some Thinkpad thing sending that message to all windows, but happening to cause this assertion because we're showing the assertion dialog? Anyway, I don't understand what ipc/glue/WindowsMessageLoop.cpp is for: is it for messages bubbled up from the plugin to the browser's process? I'm not really sure what the assertion means, or whether I should just be adding a case in that message loop's switch to pass it on to the window. Jim, can you explain?
Reporter | ||
Comment 2•10 years ago
|
||
With this patch we can also remove the ::ShowWindow() calls that were added in bug 626813.
Reporter | ||
Comment 3•10 years ago
|
||
jimm ^
Reporter | ||
Comment 4•10 years ago
|
||
Try build for this patch, if anyone wants to test it: https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/cmccormack@mozilla.com-e6fe32a87603/
Tested on Win7 x64, Synaptics driver v15.2.20.0, nppdf32.dll v9.5.0.270: - fg window moves correctly, even with pdf open in bg and embedded pdf in bg - fg window with embedded pdf, only the pdf moves, no way to move the HTML page with touchpad (tested on http://people.mozilla.org/~cmccormack/misc/pdf.html) Thanks for your efforts on this! Danilo
Reporter | ||
Comment 6•10 years ago
|
||
Danilo, thanks for testing. The issue with scrolling when you have the page with the PDF in the foreground tab could be that focus is not being set away from the plugin. If the plugin has focus, then the trackpad driver is going to send it the scrolling messages. Clicking on the page background does not take away focus from the plugin (should it? I am not sure). If you click on say the location bar, then use the trackpad to scroll, does the page scroll or the plugin? On my machine, that will cause the page to scroll.
Yeah, forgot to mention, but I did already test this: - restart in safe mode - focus the location bar -> touchpad scrolls pdf - tab -> focus to the search bar -> touchpad scrolls pdf - tab -> focus to the page -> touchpad scrolls pdf - tab -> focus to the tab "tab" -> touchpad scrolls pdf - select some text on page -> touchpad scrolls pdf Truth be said, this is the same behaviour I also experience viewing your page with IE 9.0.8112.16421.
Comment 8•10 years ago
|
||
Re comment 6, it seems to me that if I click outside the plugin, I want that plugin to lose focus. This is similar to how if I click outside of a text box and scroll, the page scrolls and not the text box. (Well, most of the time -- if the mouse hovers over the text box then the text box scrolls...) Otherwise, if I understand you correctly, it would be impossible to scroll a page containing a pdf with the mousewheel, correct? There aren't too many pages that do that, but for instance bank pages that show your statement in a pdf with other information around it... Thanks for your work on this -- I'm looking forward to it landing :)
Reporter | ||
Comment 9•10 years ago
|
||
Danilo, since you said you're using v15.2.x.y of the Synaptics driver, and all of the Synaptics hacks we have currently apply automatically only for versions <= 15.0.x.y, so you'll need to force it on. Try going to about:config and setting ui.trackpoint_hack.enabled from -1 (the default) to 1 (which should force it on), then restart the browser.
Comment 10•10 years ago
|
||
Ok, now it works as intended! How it now works: - fg html page with bg pdf -> fg html scrolls - fg pdf -> pdf scrolls - fg html page with embedded pdf (follow steps in order): . focus location bar -> html page scrolls . focus search bar, etc. -> html page scrolls . select page text -> html page scrolls . hover emb pdf area -> nothing scrolls (I suppose that now the acrobat plugin *toolbars* have focus) . click acrobat plugin scrollbars/pdf page -> pdf scrolls . hover *off* emb pdf area -> pdf scrolls . click anything outside pdf (html page, location bar, etc) -> html page scrolls Only one weird thing left: - open new tab - go to http://people.mozilla.org/~cmccormack/misc/pdf.html - do *not* hover mouse over emb pdf area -> pdf scrolls - hover mouse over emb pdf area -> pdf scrolls - click html page -> pdf scrolls - select text on html page -> pdf scrolls - move html page vertical scrollbar down/up -> pdf scrolls - focus location bar -> html page scrolls - from here on, "How it now works" behaviour resumes
![]() |
||
Comment 11•10 years ago
|
||
Cam, are you ready to kick off review?
Reporter | ||
Comment 12•10 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #11) > Cam, are you ready to kick off review? I should fix the assertions I trigger in comment 1. Can you suggest how to do that?
![]() |
||
Comment 13•10 years ago
|
||
> Anyway, I don't understand what ipc/glue/WindowsMessageLoop.cpp is for: is > it for messages bubbled up from the plugin to the browser's process? I'm > not really sure what the assertion means, or whether I should just be adding > a case in that message loop's switch to pass it on to the window. Jim, can > you explain? It's a trap for messages that might trigger ipc events. Our ipc protocol requires that when a process is waiting on a response to a sent message, it can't send another message over the wire. So whenever a process is in this situation, it sets a hook that traps and defers any incoming windowing messages until a response from the other process is received. If your patch is triggering the non-queued warning, you should add a message handler in WindowsMessageLoop for whatever event is being received. The warning indicates the WindowsMessageLoop hook doesn't know how to handle the message. There are a number of handlers already defined, WM_VSCROLL looks like it can use the standard DeferredSendMessage message wrapper - http://mxr.mozilla.org/mozilla-central/source/ipc/glue/WindowsMessageLoop.cpp#203
Reporter | ||
Comment 14•10 years ago
|
||
Thanks, that seems to work. I added a case there for WM_KEYDOWN, too, as I noticed that was sometimes being sent when doing horizontal trackpad scrolling. I didn't get WM_KEYUP -- I suppose it got sent while we're not doing a synchronous IPC message -- but I added it too just in case. (Otherwise we might not track key down/up properly.)
Attachment #595947 -
Attachment is obsolete: true
Attachment #602577 -
Flags: review?(jmathies)
Reporter | ||
Comment 15•10 years ago
|
||
Maybe we should bump the version check for the Synaptics driver to include v15.2.0.0 as well.
Comment 16•10 years ago
|
||
FYI: bug 672175 was landed on m-i.
Reporter | ||
Comment 17•10 years ago
|
||
OK, might need to do some rebasing then.
![]() |
||
Updated•10 years ago
|
Attachment #602577 -
Flags: review?(jmathies) → review+
Reporter | ||
Comment 18•10 years ago
|
||
Since Danilo in comment 5 is reporting the need for the hack with v15.2.x.y of the driver.
Attachment #603945 -
Flags: review?(masayuki)
Reporter | ||
Comment 19•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0dba3f1cc92
Comment 20•10 years ago
|
||
Cameron: I checked with the landed patch and attachment 603945 [details] [diff] [review]. However, if I load PDF in background tab (either http://www.unicode.org/charts/PDF/U0A00.pdf or http://people.mozilla.org/~cmccormack/misc/pdf.html), I cannot scroll any normal web page in foreground tab (I tested with latest Synaptics driver, http://www.synaptics.com/node/55). And then, I can see a lot of warnings in console: WARNING: Not yet implemented!: file c:/Users/toybox/mozilla/mc-a/src/dom/plugins/ipc/PluginModuleChild.cpp, line 1285 So, I think you should back out the patch. And also the latest version of the driver is 15.2.20, so, it doesn't make sense we hardcode the latest version check. I think that we don't need the version check until the driver will fix this bug or the max version should be in prefs.
Reporter | ||
Comment 21•10 years ago
|
||
OK, that's unfortunate! I will back out. I've tested locally just with 15.0.18.0 (on Vista), and scrolling works fine in whether the PDF is in the foreground or background tab. I don't get the PluginModuleChild assertions either. I will try updating to 15.2.20 locally and see if I get the same problems as you. If you think we should remove the version check altogether, I can do that. I think we need to talk to someone at Synaptics to let them know what the best way they can detect our window and send scrolling messages to us is. I don't think the current hacking and reverse engineering is really sustainable.
Reporter | ||
Comment 22•10 years ago
|
||
Backed out: https://hg.mozilla.org/integration/mozilla-inbound/rev/46477a0c58c6
Comment 23•10 years ago
|
||
(In reply to Cameron McCormack (:heycam) from comment #21) > OK, that's unfortunate! I will back out. I've tested locally just with > 15.0.18.0 (on Vista), and scrolling works fine in whether the PDF is in the > foreground or background tab. I don't get the PluginModuleChild assertions > either. Oh, really? The pymake or something might have failed to rebiuld. I'm going to rebuild it cleanly. > If you think we should remove the version check altogether, I can do that. Please do it. > I think we need to talk to someone at Synaptics to let them know what the > best way they can detect our window and send scrolling messages to us is. I > don't think the current hacking and reverse engineering is really > sustainable. The driver seems to target the window under the mouse cursor rather than focused window. It's same as our behavior. So, if they find a window of us (except GeckoPluginWindow) in the ancestors of a window (or itself) under mouse cursor, they should send the message to the found our window. If so, we redirect the message to the descendant most window under the cursor *when* it's necessary. And also, they send WM_KEYDOWN of arrow keys for horizontal scrolling. It's too bad for us because it causes dispatching DOM key events rather than DOM wheel events. So, they should use WM_MOUSEHWHEEL message for horizontal scrolling. If they're using SendInput() it's supported on Vista or latter only. Otherwise, e.g., using SendMessage(), it can be used on XP either. We're handling WM_MOUSEHWEEL on all versions of Windows.
Comment 24•10 years ago
|
||
And maybe, they might not realize yet, WM_MOUSEWHEEL and WM_MOUSEHWHEEL are not command for scrolling. They're just notification of mouse wheel. E.g., game may use it for non-scrolling action. Something like Google Map use it for zooming in/out. So, basically, scrollbar check doesn't make sense...
Comment 25•10 years ago
|
||
I built a clean debug build, but it still prints a lot of warnings.
Reporter | ||
Comment 26•10 years ago
|
||
Are they the mozilla::plugins::child::_status warning? There might be a difference between the PDF plugins we are using. I have no idea whether it's reasonable that mozilla::plugins::child::_status reports that it is not yet implemented, or what its behaviour might be if were implemented.
Comment 27•10 years ago
|
||
A thing I noticed from the previous bug (626813): Adobe and Nitro PDF plug-ins showed this problem, but Foxit's plugin (for whatever reason) didn't show it (you could scroll using the touchpad even if a PDF was open in another tab). I emailed Foxit and I was told "Ours is a windowed' s OCX plugin." But, that discussion didn't move any further.
Comment 28•10 years ago
|
||
Sorry to "resurrect": I'm on Windows 8 Consumer Preview (NOT Release Preview...yet, hehe) x64 with Synaptics 15.0.24 and Firefox 12.0 and THE ISSUE IS GONE. I don't know if this is normal, since the original bug 626813 was not scheduled to land until FF 13.
Reporter | ||
Comment 29•9 years ago
|
||
Unassigning myself since I haven't looked at Synaptics issues for a while.
Assignee: cam → nobody
Status: ASSIGNED → NEW
Reporter | ||
Updated•9 years ago
|
Attachment #603945 -
Flags: review?(masayuki)
You need to log in
before you can comment on or make changes to this bug.
Description
•