Closed
Bug 657844
Opened 14 years ago
Closed 14 years ago
SynthesizeMouseMove calls should be disabled in fennec
Categories
(Core :: DOM: UI Events & Focus Handling, defect)
Tracking
()
RESOLVED
FIXED
mozilla7
People
(Reporter: romaxa, Assigned: romaxa)
References
Details
Attachments
(1 file, 5 obsolete files)
2.96 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
While scrolling Fennec UI remote-viewport we do call SynthesizeMouseMove from PresShell::DidDoReflow http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#7865
IIRC that is needed in order to update cursor position when FF scrolled with mouse wheel, but cursor stay on the same screen position (content behind cursor changed)....
I think this is just wasting CPU time Event handling, dispatching HitTest calls just for nothing on Mobile...
And I think we should find some conditions which help us to disable this for Mobile fennec UI process...
Assignee | ||
Comment 1•14 years ago
|
||
Assignee | ||
Comment 2•14 years ago
|
||
Not sure about other platforms which could use MOBILE_OPTIMIZE_GFX but possible having mouse pointer... like neetbooks et.c...
Probably it is better to make some pref, and control it per XUL application client.
Assignee | ||
Comment 3•14 years ago
|
||
should be more flexible for different platforms handling and different xulrunner applications
Attachment #533314 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 4•14 years ago
|
||
ups, typo bug
Attachment #533314 -
Attachment is obsolete: true
Attachment #533314 -
Flags: review?(bzbarsky)
Attachment #533317 -
Flags: review?(bzbarsky)
Comment 5•14 years ago
|
||
Comment on attachment 533311 [details] [diff] [review]
Disable synthMouseMove for mobile
Hmm. Why is a gfx-specific define the right thing to condition this on?
Assignee | ||
Comment 6•14 years ago
|
||
Comment on attachment 533311 [details] [diff] [review]
Disable synthMouseMove for mobile
yep, I think GFX define is bad way to disable it and not flexible...
Attachment #533311 -
Flags: review?(bzbarsky)
Comment 7•14 years ago
|
||
Comment on attachment 533317 [details] [diff] [review]
Pref version of synthMouseMove disable
This looks fine, but why not just make that a live pref using a bool pref cache?
Attachment #533317 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 8•14 years ago
|
||
Attachment #533311 -
Attachment is obsolete: true
Attachment #534080 -
Flags: review?(bzbarsky)
Comment 9•14 years ago
|
||
Comment on attachment 534080 [details] [diff] [review]
Version with cacheBool var
This is going to add a new cache on each DidReflow call.
Attachment #534080 -
Flags: review?(bzbarsky) → review-
Assignee | ||
Comment 10•14 years ago
|
||
Did not found any static initialize function in PresShell, so I hope it is ok to reuse static condition in PresShell ctor. registeredReporter
Attachment #534080 -
Attachment is obsolete: true
Attachment #534135 -
Flags: review?(bzbarsky)
Comment 11•14 years ago
|
||
Comment on attachment 534135 [details] [diff] [review]
Version with cacheBool var v2
r=me
Attachment #534135 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Updated•14 years ago
|
Attachment #533317 -
Attachment is obsolete: true
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Comment 12•14 years ago
|
||
Keywords: checkin-needed
Whiteboard: fixed-in-cedar
Comment 13•14 years ago
|
||
The push from comment 12 appears to have caused orange in mochitest-chrome on all platforms:
> layout/style/test/chrome/test_hover.html | Test timed out.
Sample log:
http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1306088180.1306089707.19816.gz
http://tbpl.mozilla.org/?tree=Cedar&rev=dfeab223be6d
Backed out:
http://hg.mozilla.org/projects/cedar/rev/dfeab223be6d
Whiteboard: fixed-in-cedar
Comment 14•14 years ago
|
||
(In reply to comment #13)
> Backed out:
> http://hg.mozilla.org/projects/cedar/rev/dfeab223be6d
er sorry, that's the original push. backout push was:
http://hg.mozilla.org/projects/cedar/rev/8176003df609
Comment 15•14 years ago
|
||
If the pref isn't specified does it maybe default to returning false?
Comment 16•14 years ago
|
||
Er, yes. There's an optional third argument (I hate optional arguments) for what the default value should be, defaulting to false. I should have caught that... :(
Assignee | ||
Comment 17•14 years ago
|
||
Hm, I saw in other implementations static variable initially initialized with some default value, and I though that AddBoolPrefVarCache will take value pointer and check it's value and set it as default (we had this before somewhere in GTK API)..
But ok, here is one more default value passed
Attachment #534135 -
Attachment is obsolete: true
Attachment #534436 -
Flags: review?(bzbarsky)
Updated•14 years ago
|
Attachment #534436 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Comment 18•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Updated•6 years ago
|
Component: Event Handling → User events and focus handling
You need to log in
before you can comment on or make changes to this bug.
Description
•