471325 - Focus ring on native-styled tab is cut off at right

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Stefan [:stefanh]]

Attachment: Testcase

(Minefield trunk/1.9.1, Mac OS 10.5.6)

STR:

1) Load testcase
2) Hit the "Focus first tab" button
3) Look at the focus ring

Note: Focus ring on rightmost tab looks ok (you can get there by clicking on the tab once you have focus on the first tab).

Comment 1

[Markus Stange [:mstange]]

Is there a way to focus tabs other than calling .focus()?

I tried to fix this bug by setting position:relative on the selected tab, but that seems to break the frame order... bug 351238?

Comment 2

[Stefan [:stefanh]]

(In reply to comment #1)
> Is there a way to focus tabs other than calling .focus()?

Hmm, I don't know. I noticed it because comm-central/source/suite/browser/pageinfo/pageInfo.xul focus the General tab upon onload with .focus() in comm-central/source/suite/browser/pageinfo/pageInfo.js#341.

I'm not saying this should be fixed because of that, but I can imagine that a bunch of other apps/extensions would want to do (or does) something similar. It feels like a "natural" way to focus a tab under certain circumstances.

> I tried to fix this bug by setting position:relative on the selected tab, but
> that seems to break the frame order... bug 351238?

Hmm, comments seems to indicate that bug is wfm? If there is no reasonable way to fix this, maybe we should make it so the focus ring doesn't appear at all?

Comment 3

[Josh Aas]

I don't think I see this bug, can we get a screenshot attached here?

Comment 4

[Phil Ringnalda (:philor)]

Attachment: Screenshot of testcase

Screenshot on my trunk build from 20090101.

Comment 5

[Markus Stange [:mstange]]

tab:focus {
  opacity: 0.99;
}

might work.

Comment 6

[Markus Stange [:mstange]]

Attachment: fix

It does work.

Comment 7

[Dão Gottwald [:dao]]

(In reply to comment #5)
> tab:focus {
>   opacity: 0.99;
> }
> 
> might work.

Looks like a hack to me. What's the underlying problem here?

Comment 8

[Markus Stange [:mstange]]

The focus ring extend's beyond the focused tab's rect (it's in the overflow area), but it's partly hidden by the tab right next to it. The right tab is on top of it because it's later in the document.
So we need to manipulate the natural stacking order - the most natural way to do that is (to me) using position:relative, but that fails spectacularly. Setting opacity is the other way I know of that changes the stacking order.

Comment 9

[Markus Stange [:mstange]]

The focus ring extend's beyond the focused tab's rect (it's in the overflow area), but it's partly hidden by the tab right next to it. The right tab is on top of it because it's later in the document.
So we need to manipulate the natural stacking order - the most natural way to do that is (to me) using position:relative, but that fails spectacularly. Setting opacity is the other way I know of that changes the stacking order.

Comment 10

[Markus Stange [:mstange]]

Attachment: screenshot with position:relative

This is what happens when I set position:relative on the focused tab - something with the frame order goes wrong. It looks like the flexed centering spacer and the focused tab swap places.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

looks like position:relative is screwing up XUL. That might be easy to fix. Reduced testcase?

Comment 12

[Josh Aas]

Would like a fix but this bug isn't serious enough to block on.

Comment 13

[Dão Gottwald [:dao]]

(In reply to comment #9)
> Setting opacity is the other way I know of that changes the stacking order.

Does the CSS spec say that or is it a Gecko quirk?

Comment 14

[Markus Stange [:mstange]]

The CSS spec says that, see bug 317375 comment 32 and http://archivist.incutio.com/viewlist/css-discuss/69890

Comment 15

[Dão Gottwald [:dao]]

Is a 1% transparency so faint that it's definitely not visible? I'm thinking of the selected tabbrowser tab which would be subject to this change, and which is supposed to connect with the above toolbar.

Comment 16

[Markus Stange [:mstange]]

(In reply to comment #15)
Oh, good thinking. I've just checked the tab background (#969696); the RGB values that end up on the screen haven't changed with the opacity, so it's definitely not visible.

(In reply to comment #11)
> looks like position:relative is screwing up XUL. That might be easy to fix.
> Reduced testcase?

I'll try to make one tomorrow.

Comment 17

[Dão Gottwald [:dao]]

Comment on attachment 355822 [details] [diff] [review]
fix

waiting for position:relative testcase

Comment 18

[Markus Stange [:mstange]]

Attachment: fix with position:relative

The problem with position:relative seems to have been fixed by bug 307394.

Comment 19

[Markus Stange [:mstange]]

pushed: http://hg.mozilla.org/mozilla-central/rev/3615bdf56262