Last Comment Bug 757368 - moving the caret with arrow keys don't work any more in editor if a floating panel contains a focusable element
: moving the caret with arrow keys don't work any more in editor if a floating ...
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: neil@parkwaycc.co.uk
:
: Makoto Kato [:m_kato]
Mentors:
Depends on:
Blocks: 669026
  Show dependency treegraph
 
Reported: 2012-05-22 02:42 PDT by Daniel Glazman (:glazou)
Modified: 2012-05-31 02:36 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Possible patch (-w) (1.80 KB, patch)
2012-05-23 04:36 PDT, neil@parkwaycc.co.uk
no flags Details | Diff | Splinter Review
Proposed patch (1.13 KB, patch)
2012-05-23 07:57 PDT, neil@parkwaycc.co.uk
enndeakin: review+
Details | Diff | Splinter Review

Description Daniel Glazman (:glazou) 2012-05-22 02:42:40 PDT
This bug is a regression introduced by fix for bug 669026 (verified on OS X
between revision ede336ccaed0 - working - and revision 4d0391866459 - failing).

If I launch BlueGriffon 1.4 and create a blank document, type
a few chars and open the CSS Properties panel (that is a XUL floating panel),
using the left arrow to move the caret works fine in the editor.
If I do the same with BlueGriffon 1.4.1 or even more recent (fix for the
bug 669026 is in), then the first menulist in my CSS Properties panel acquires
the focus when the left arrow key is pressed and the editor loses the focus! I
have a lot of users' feedback about this, and this is bad enough to trigger a
new minor version of BlueGriffon with a fix for that wrong behaviour.
Comment 1 Daniel Glazman (:glazou) 2012-05-22 23:45:15 PDT
Bug confirmed on Windows (XP, 7, 8preview) and Linux.
Comment 2 neil@parkwaycc.co.uk 2012-05-23 03:32:53 PDT
http://mxr.mozilla.org/comm-central/source/mozilla/dom/base/nsFocusManager.cpp#2436
// if there is no focus, yet a panel is open, focus the first item in
// the panel
Comment 3 neil@parkwaycc.co.uk 2012-05-23 04:36:53 PDT
Created attachment 626392 [details] [diff] [review]
Possible patch (-w)

I'm not sure what this code is trying to achieve, otherwise I'd remove it ;-)
Comment 4 Neil Deakin 2012-05-23 05:03:53 PDT
That code is needed so that pressing tab when a panel is open cycles into the panel itself. This patch seems to make it so that navigation into the panel isn't possible.

From the bug description above, it seems that DetermineElementToMoveFocus is being called with MOVEFOCUS_CARET. If that's the case, the popup checking step could just be skipped.
Comment 5 Daniel Glazman (:glazou) 2012-05-23 05:51:31 PDT
(In reply to Neil Deakin from comment #4)

> That code is needed so that pressing tab when a panel is open cycles into
> the panel itself. This patch seems to make it so that navigation into the
> panel isn't possible.

I have just tested Neil's patch in BlueGriffon:

  1. it solves the bug
  2. cycling into a panel still works as usual if the focus is already in the
     panel

Were you saying that if the focus is in the last tabbable element in the main
window, a tab should switch to the first available panel containing a tabbable
element?
Comment 6 neil@parkwaycc.co.uk 2012-05-23 07:57:47 PDT
Created attachment 626441 [details] [diff] [review]
Proposed patch

Sure, that seems to work too.
Comment 7 neil@parkwaycc.co.uk 2012-05-31 02:36:13 PDT
Pushed mozilla-central changeset f6d082275253.

Note You need to log in before you can comment on or make changes to this bug.