After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 84215 - No visible focus indication for Workspace Switcher applet
No visible focus indication for Workspace Switcher applet
Status: RESOLVED FIXED
Product: gnome-panel
Classification: Other
Component: libpanel-applet
1.5.x
Other All
: Normal major
: ---
Assigned To: Mark McLoughlin
Panel Maintainers
: 92039 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2002-06-05 08:41 UTC by padraig.obriain
Modified: 2004-12-22 21:47 UTC
See Also:
GNOME target: ---
GNOME version: 2.0


Attachments
Proposed patch (5.88 KB, patch)
2002-07-17 12:32 UTC, padraig.obriain
none Details | Review

Description padraig.obriain 2002-06-05 08:41:52 UTC
We need a visible focus indication when focus is on an applet.

The attempt to draw a dotted line focus indication around the applet has
been removed as it caused a number of bugs, 83254, 83060, 83607.
Comment 1 Luis Villa 2002-06-06 19:23:34 UTC
I've still got visible focus indications around all of my panel
applets, FWIW.
Comment 2 padraig.obriain 2002-06-07 09:01:14 UTC
I do not see any focus indication around Workspace Switcher applet; I
am updating summary.
Comment 3 padraig.obriain 2002-06-07 09:06:57 UTC
I also cannot see focus indication for cdplayer.
Comment 4 padraig.obriain 2002-06-21 09:41:53 UTC
I have changewd the Prioriy to Normal to reflect that fixing this bug
is likely to be hard.
Comment 5 padraig.obriain 2002-07-17 12:32:33 UTC
Created attachment 9924 [details] [review]
Proposed patch
Comment 6 padraig.obriain 2002-07-17 12:37:32 UTC
The attached patch attempts to draw focus indication around applets as
unobtrusively as possible.

The chnage should have no impact on the size of the panel or on
applets which do not receive focus. For applets which receive focus,
if the applet requests a size within two pixels of the size of the
panel the size allocated to the applet will be reduced by one or two
pixels so that there is room for the focus indicator. Thus, on a 24
pixel panel the size of the Workspace Switcher is reduced from 20
pixels to 18; the other 4 (2x2) pixels are used for a frame.
Comment 7 Mark McLoughlin 2002-07-18 00:49:42 UTC
Hi Padraig. I'm a little nervous about this patch for two reasons

1) You're reducing the size allocated to some applets which may be
assuming that their size is panel_applet_frame_get_size (applet) - 4.
I'm not sure whether there are any applets assuming that are there ?
But, because there might be - I'd prefer to not take the approach of
reducing any applets allocation until we start developing for 2.2.

Aside: The applet would be broken in this assumption as the size
property is only meant as an indication by which to decide on how to
structure the applets widgets, not as an indication of the panel's
actual size. Applets should only use the 'size' property to layout its
widgets (e.g. how many rows of widgets) but still honour the size
allocated to it in size_allocate.

2) In the patch you are also making the same incorrect assumption
above with 

+        if (requisition->width > applet->priv->size)
+                requisition->width = applet->priv->size;


Another point specifically about the Workspace Switcher - it doesn't
seem to navigable with the keyboard at all - is that not more important ?

Comment 8 padraig.obriain 2002-07-18 10:33:40 UTC
Mark, Thanks for the review.

The alternative to reducing the size allocated to an applet is to
allow the size of the panel to increase. I had understood that this
was less desirable. What do you think?

Another apporach would be to deal with the problem on an
applet-by-applet basis and make it the applet's responsibility to
leave room for two pixel focus indication if the applet can focus.

With panel size of 24, the clock applet requests a height of 22 so
there is room for the focus indicator.

For the Workspace Switcher applet, if the variable internal_size in
pager_update in pager.c were set to pager->size - 2 * 3, there would
be room for the focus indicator.

For the cdplayer applet with panel size of 24 the requested height is
24 so there is no room for the focus indicator unless the height
allocated is less than the requested height.

As regards the navigability with the keyboard of the Workspace
Switcher applet, my understanding is that as long as its
functionaility can be provided in another way via the keyboard, we are
not required to make it navigable. I do not recall the details but my
understanding is that switching workspaces and focusing windows in
other workspaces can be done using the keyboard. 
Comment 9 Calum Benson 2002-09-02 11:00:56 UTC
Latest status guys?  This is on our 'must fix for FCS' list, and there
are still some applets that are showing no focus at all (e.g. bug
#92039 and bug #92040)
Comment 10 padraig.obriain 2002-09-02 14:46:30 UTC
*** Bug 92039 has been marked as a duplicate of this bug. ***
Comment 11 Mark McLoughlin 2002-10-16 23:58:13 UTC
Okay - fixed by:

2002-10-17  Mark McLoughlin  <mark@skynet.ie>

        Patch from Padraig to ensure there is enough room around
        each applet to draw a focus indication if one is needed.
        Focus indication is only needed when there is a toplevel
        tooltip on the PanelApplet *or* the applet has no
        focusable children. Fixes #84215.

        Please read:

http://mail.gnome.org/archives/desktop-devel-list/2002-October/msg00227.html

        before deciding that this behaviour is wrong.

        * panel-applet.c:
        (panel_applet_can_focus): only focus if the applet has
        a toplevel tooltip or no focusable children.
        (panel_applet_size_request): request room to draw the
        focus indication. Deliberately ignore focus padding.
        (panel_applet_size_allocate): adjust the child's position
        to give room to draw focus.
        (panel_applet_expose): draw focus here.
        (panel_applet_class_init): hook up.