moving the caret with arrow keys don't work any more in editor if a floating panel contains a focusable element

RESOLVED FIXED

Status

()

Core
Editor
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: glazou, Assigned: neil@parkwaycc.co.uk)

Tracking

({regression})

Trunk
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

1.13 KB, patch
Neil Deakin (mostly unavailable until September)
: review+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
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.
Blocks: 669026
(Reporter)

Comment 1

5 years ago
Bug confirmed on Windows (XP, 7, 8preview) and Linux.
(Assignee)

Comment 2

5 years ago
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
(Assignee)

Comment 3

5 years ago
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 ;-)
Attachment #626392 - Flags: feedback?(enndeakin)
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.
(Reporter)

Comment 5

5 years ago
(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?
(Assignee)

Comment 6

5 years ago
Created attachment 626441 [details] [diff] [review]
Proposed patch

Sure, that seems to work too.
Attachment #626392 - Attachment is obsolete: true
Attachment #626392 - Flags: feedback?(enndeakin)
Attachment #626441 - Flags: review?(enndeakin)
Attachment #626441 - Flags: review?(enndeakin) → review+
(Assignee)

Comment 7

5 years ago
Pushed mozilla-central changeset f6d082275253.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.