Closed
Bug 715518
Opened 12 years ago
Closed 12 years ago
WASD controls don't work in 3D view if typeaheadfind is active
Categories
(DevTools :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 12
People
(Reporter: smaug, Assigned: vporof)
Details
(Whiteboard: [tilt][fixed-in-fx-team])
Attachments
(1 file, 3 obsolete files)
7.99 KB,
patch
|
rcampbell
:
review+
|
Details | Diff | Splinter Review |
If the "search for text when I start typing" is active, using W-A-S-D tries to just find some text in the page, and controlling the 3D view doesn't work.
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → vporof
Whiteboard: tilt]
Assignee | ||
Updated•12 years ago
|
Whiteboard: tilt] → [tilt]
Assignee | ||
Updated•12 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #588512 -
Flags: review?(rcampbell)
Assignee | ||
Comment 2•12 years ago
|
||
On a closer look, it seems that this bug isn't limited only on WASD keys. For example, the functionality introduced by bug 712029 relies on the R key. Or, for zooming, we'd have to use + - or I O keys, and that would also trigger the Quick Find search box when typeahead is enabled. Added a patch fixing every possible use case, and avoiding stuff like this to happen in the future.
Attachment #588512 -
Attachment is obsolete: true
Attachment #588512 -
Flags: review?(rcampbell)
Attachment #588548 -
Flags: review?(rcampbell)
Assignee | ||
Comment 3•12 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=8540877&tree=Try TEST-INFO | chrome://mochitests/content/browser/browser/devtools/tilt/test/browser_tilt_arcball.js | Console message: [JavaScript Warning: "WebGL: Can't get a usable WebGL context" {file: "resource:///modules/devtools/TiltGL.jsm" line: 1606}] TEST-INFO | chrome://mochitests/content/browser/browser/devtools/tilt/test/browser_tilt_arcball.js | Console message: [JavaScript Warning: "WebGL: Can't get a usable WebGL context" {file: "resource:///modules/devtools/TiltGL.jsm" line: 1606}] TEST-INFO | chrome://mochitests/content/browser/browser/devtools/tilt/test/browser_tilt_arcball.js | Console message: [JavaScript Error: "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMHTMLCanvasElement.getContext]"] TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/tilt/test/browser_tilt_arcball.js | application timed out after 330 seconds with no output I know the problem: I'll need to test for WebGL first (like in all other tests). Only a small part of the test will require this.
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #588548 -
Attachment is obsolete: true
Attachment #588548 -
Flags: review?(rcampbell)
Attachment #588726 -
Flags: review?(rcampbell)
Assignee | ||
Comment 5•12 years ago
|
||
Cleaned up. https://tbpl.mozilla.org/?tree=Try&rev=99f836e9fef1
Attachment #588726 -
Attachment is obsolete: true
Attachment #588726 -
Flags: review?(rcampbell)
Attachment #588814 -
Flags: review?(rcampbell)
Comment 6•12 years ago
|
||
Comment on attachment 588814 [details] [diff] [review] v4 +++ b/browser/devtools/tilt/TiltVisualizer.jsm @@ -994,14 +994,12 @@ { let code = e.keyCode || e.which; - if (code >= e.DOM_VK_LEFT && code <= e.DOM_VK_DOWN) { + if (!e.altKey && !e.ctrlKey && !e.metaKey && !e.shiftKey) { e.preventDefault(); e.stopPropagation(); - } - if (e.altKey || e.ctrlKey || e.metaKey || e.shiftKey) { + this.arcball.keyDown(code); + } else { this.arcball.cancelKeyEvents(); - } else { - this.arcball.keyDown(code); } }, alright, lemme see if I've got this: You're checking that the event doesn't include a modifier key, and if so, you preventDefault, stopPropagation and then send the key code the arcball controller. And then cancel them? This only happens when the tilt context has focus or does this capture all key events? Like on the breadcrumbs and HTML panel too?
Assignee | ||
Comment 7•12 years ago
|
||
*head spinning* I think you got this wrong. The code would be if (!e.altKey && !e.ctrlKey && !e.metaKey && !e.shiftKey) { e.preventDefault(); e.stopPropagation(); this.arcball.keyDown(code); } else { this.arcball.cancelKeyEvents(); } First of all, this line a bit above would help: canvas.addEventListener("keydown", this.onKeyDown, false); Same goes for all other events. So, obviously, I'm catching events only on the canvas overlay when it has focus. Second of all: * check if a modifier key is pressed; if not, tilt is king and proceed with what the arcball needs to do. (the modifier key scenario avoids tons of unfiled bugs which we can talk about on irc). * if a modifier key *is* pressed, cancel all key actions happening in the arcball This isn't a thing I fixed here, it was an older fix. I'm rewording "if (e.altKey || e.ctrlKey || e.metaKey || e.shiftKey)" to "if (!e.altKey && !e.ctrlKey && !e.metaKey && !e.shiftKey)" because i cleaned up some code in those key event handlers in this particular bug (no more "if (code >= e.DOM_VK_LEFT && code <= e.DOM_VK_DOWN)"). Talk to me on IRC.
Comment 8•12 years ago
|
||
(In reply to Victor Porof from comment #7) > *head spinning* > > I think you got this wrong. The code would be quite prossibly! > if (!e.altKey && !e.ctrlKey && !e.metaKey && !e.shiftKey) { > e.preventDefault(); > e.stopPropagation(); > this.arcball.keyDown(code); > } else { > this.arcball.cancelKeyEvents(); > } > > First of all, this line a bit above would help: > canvas.addEventListener("keydown", this.onKeyDown, false); > > Same goes for all other events. So, obviously, I'm catching events only on > the canvas overlay when it has focus. Ah, ok, that's helpful. A day of code reviews makes rob a something-something. Should've looked up the originating method. > Second of all: > * check if a modifier key is pressed; if not, tilt is king and proceed with > what the arcball needs to do. (the modifier key scenario avoids tons of > unfiled bugs which we can talk about on irc). clown'll eat me. > * if a modifier key *is* pressed, cancel all key actions happening in the > arcball > > This isn't a thing I fixed here, it was an older fix. I'm rewording > "if (e.altKey || e.ctrlKey || e.metaKey || e.shiftKey)" to > "if (!e.altKey && !e.ctrlKey && !e.metaKey && !e.shiftKey)" > because i cleaned up some code in those key event handlers in this > particular bug (no more "if (code >= e.DOM_VK_LEFT && code <= > e.DOM_VK_DOWN)"). > > Talk to me on IRC. OK.
Updated•12 years ago
|
Attachment #588814 -
Flags: review?(rcampbell) → review+
Updated•12 years ago
|
Whiteboard: [tilt] → [tilt][land-in-fx-team]
Comment 9•12 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/0f664cd835ec
Whiteboard: [tilt][land-in-fx-team] → [tilt][fixed-in-fx-team]
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0f664cd835ec
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 12
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•