Closed
Bug 1304692
Opened 8 years ago
Closed 8 years ago
[Mac] PointerLock doesn't work properly while in Full Screen on MacBook Pro Retina
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla52
People
(Reporter: cbadau, Assigned: xidorn)
Details
Attachments
(1 file)
[Affected versions]: - Firefox 50 Beta 1 [Affected platforms]: - Mac OS X 10.11 (MacBook Pro Retina) [Steps to reproduce]: 1. Navigate to one of the following games/demos: https://kripken.github.io/BananaBread/wasm-demo/index.html, http://substack.net/projects/voxel-forest/, http://media.tojicode.com/q3bsp/. 2. From Hamburger menu, select "Full Screen" . 3. Click on the demo and start navigating/playing. [Expected result]: - There are no issues encountered while playing/navigating. [Actual result]: - The game/demo doesn't work properly while in Full Screen. After clicking on the demo/game (to lock the pointer), the game/demo is blocked in a position and can't be moved from there. [Regression range]: - It is not a regression, it reproduces since the feature landed in Firefox (Nightly 50.0a1: 2016-07-10). [Additional notes]: - The issue is reproducible only on MacBook Pro Retina 10.11.6 -> Graphics: Intel Iris Pro 1536 MB - The issue is NOT reproducible on Mac OS X 10.11 -> graphics: NVIDIA GeForce GT 640M - The issue is NOT reproducible on iMac (21.5-inch, Mid 2011) 10.10.5 -> graphics: AMD Radeon HD 6750M 512 MB
Hi Andrew, SoftVision found this bug during 50.b1 sign off testing. Could you please help find an owner to investigate? PointerLock API is a feature tracking for release 50.
Flags: needinfo?(overholt)
Assignee | ||
Comment 3•8 years ago
|
||
What? It's system version related and hardware related? I'm afraid I probably don't have device for debugging this... I only have a 2013 15-inch rMBP with OS X 10.10... FWIW, the PointerLock API has been implemented since long ago. 50 just unprefix the API. I believe it is not a regression, so it has been existing for quite long... (Probably since 10.11 released?) If this issue is graphics card related, I would also guess it is some bug (maybe os bug) in the graphics part... PointerLock doesn't really involve graphics things. The only possible related thing is hiding cursor...
Assignee | ||
Comment 4•8 years ago
|
||
cbadau, is that related to e10s? If you open a non-e10s window and put it into fullscreen, does it have the same issue? (FWIW, I observe another issue on OS X with pointerlock in fullscreen with e10s enabled, which seems to have the same symptom as Linux HiDPI issue descibed in bug 1255538.)
Flags: needinfo?(xidorn+moz) → needinfo?(camelia.badau)
Reporter | ||
Comment 5•8 years ago
|
||
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #4) > cbadau, is that related to e10s? If you open a non-e10s window and put it > into fullscreen, does it have the same issue? > > (FWIW, I observe another issue on OS X with pointerlock in fullscreen with > e10s enabled, which seems to have the same symptom as Linux HiDPI issue > descibed in bug 1255538.) I can't tested on a non-e10s window because the "Open [Non-]e10s Window" option was removed from Menu (see bug 1003313). I've manually disabled e10s (set browser.tabs.remote.autostart.2 to false) and with e10s disabled, the issue is NOT reproducible.
Flags: needinfo?(camelia.badau)
Assignee | ||
Comment 6•8 years ago
|
||
Let me confirm, is the problem that, the position would move upwards at beginning, and then get stuck at the upmost point? If you try using https://mdn.github.io/pointer-lock-demo/, the red ball would go upwards consistently in that case, right? If yes, I probably know what the problem is, and have a potential fix now. But that still needs some more investigation.
Flags: needinfo?(camelia.badau)
Comment hidden (mozreview-request) |
Comment 8•8 years ago
|
||
mozreview-review |
Comment on attachment 8795168 [details] Bug 1304692 - Make puppet widget get coordinate rounding from parent. https://reviewboard.mozilla.org/r/81296/#review79922 Do we end up updating the rounding value when window is moved to another display? ::: dom/ipc/TabChild.h:577 (Diff revision 1) > nsCOMPtr<nsIDocShell> docShell = do_QueryInterface(webNav); > return GetFrom(docShell); > } > > virtual bool RecvUIResolutionChanged(const float& aDpi, > + const int32_t& aRounding, So here, rounding is the 2nd param... ::: dom/ipc/TabParent.cpp:1034 (Diff revision 1) > TryCacheDPIAndScale(); > // If mDPI was set to -1 to invalidate it and then TryCacheDPIAndScale > // fails to cache the values, then mDefaultScale.scale might be invalid. > // We don't want to send that value to content. Just send -1 for it too in > // that case. > - Unused << SendUIResolutionChanged(mDPI, mDPI < 0 ? -1.0 : mDefaultScale.scale); > + Unused << SendUIResolutionChanged( ...but here it is sent as 3rd param
Attachment #8795168 -
Flags: review?(bugs) → review-
Assignee | ||
Comment 9•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8795168 [details] Bug 1304692 - Make puppet widget get coordinate rounding from parent. https://reviewboard.mozilla.org/r/81296/#review79922 I searched mDefaultScale in the touched files, and added rounding with it, so I suppose we do update the rounding value as far as scale value is updated. Isn't that supposed to be done via UIResolutionChanged message?
Assignee | ||
Comment 10•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8795168 [details] Bug 1304692 - Make puppet widget get coordinate rounding from parent. https://reviewboard.mozilla.org/r/81296/#review79922 > ...but here it is sent as 3rd param Ah... my bad.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 12•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8795168 [details] Bug 1304692 - Make puppet widget get coordinate rounding from parent. https://reviewboard.mozilla.org/r/81296/#review79922 I tested that the rounding value is updated as expected when window is moved between monitors.
Reporter | ||
Comment 13•8 years ago
|
||
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #6) > Let me confirm, is the problem that, the position would move upwards at > beginning, and then get stuck at the upmost point? > > If you try using https://mdn.github.io/pointer-lock-demo/, the red ball > would go upwards consistently in that case, right? > > If yes, I probably know what the problem is, and have a potential fix now. > But that still needs some more investigation. Yes, that's the issue.
Flags: needinfo?(camelia.badau)
Comment 14•8 years ago
|
||
mozreview-review |
Comment on attachment 8795168 [details] Bug 1304692 - Make puppet widget get coordinate rounding from parent. https://reviewboard.mozilla.org/r/81296/#review79980 ::: dom/ipc/TabChild.cpp:2952 (Diff revision 2) > void > +TabChild::GetWidgetRounding(int32_t* aRounding) > +{ > + *aRounding = -1; > + if (!mRemoteFrame) { > + return; Do we really want to return -1 here. I think we want 1, so that PuppetWidget's code end up using 1 and not -1 ::: dom/ipc/TabChild.cpp:2960 (Diff revision 2) > + *aRounding = mRounding; > + return; > + } > + > + // Fallback to a sync call if needed. > + SendGetWidgetRounding(aRounding); Do we actually end up doing this sync call? Hopefully not normally at least. Please test that opening a new tab doesn't trigger this case.
Attachment #8795168 -
Flags: review?(bugs) → review+
Tracked for Fx50 based on SV recommendation.
tracking-firefox50:
--- → +
Comment hidden (mozreview-request) |
Assignee | ||
Comment 17•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8795168 [details] Bug 1304692 - Make puppet widget get coordinate rounding from parent. https://reviewboard.mozilla.org/r/81296/#review79980 > Do we really want to return -1 here. I think we want 1, so that PuppetWidget's code end up using 1 and not -1 Probably makes sense, although it doesn't really matter, since -1 is effectively the same as 1 when used as a rounding factor :) > Do we actually end up doing this sync call? Hopefully not normally at least. Please test that opening a new tab doesn't trigger this case. I tested it via adding a MOZ_ASSERT_UNREACHABLE before this statement, and it was never reached, so supposingly it is never called.
Comment 18•8 years ago
|
||
Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/853d4b8a9fde Make puppet widget get coordinate rounding from parent. r=smaug
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → xidorn+moz
Comment 19•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/853d4b8a9fde
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment 20•8 years ago
|
||
This landed in nightly a couple of weeks ago, and is tracked for fx 50. Xidorn, if you think this is ready to be uplifted would you mind filing the request for aurora/beta?
Flags: needinfo?(xidorn+moz)
Assignee | ||
Comment 21•8 years ago
|
||
I'm not confident on uplifting this. Let's have it ride the train. I think it is an issue with e10s, but not a new issue as itself.
Flags: needinfo?(xidorn+moz)
Comment 22•8 years ago
|
||
Wontfix for fx 50 per comment 21.
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•