251589 - XUL tab focus indication should appear inside the tab

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Aaron Leventhal]

When XUL tabs get focused (they do as of bug 175893) the focus outline is
currently appearing outside of the tab (because of fix for bug 151375). It
should appear inside.

Comment 1

[Aaron Leventhal]

Attachment: Use hbox to surround tab's anonymous image and label, put outline on that

Comment 2

[Aaron Leventhal]

Comment on attachment 153312 [details] [diff] [review]
Use hbox to surround tab's anonymous image and label, put outline on that

Will need similar patch for seamonkey.

Comment 3

[Mike Connor [:mconnor]]

Comment on attachment 153312 [details] [diff] [review]
Use hbox to surround tab's anonymous image and label, put outline on that

r=me, no sr needed

Comment 4

[Aaron Leventhal]

Fixed for Firefox, will leave open for Seamonkey work.

Checking in content/widgets/tabbox.xml;
/cvsroot/mozilla/toolkit/content/widgets/tabbox.xml,v  <--  tabbox.xml
new revision: 1.12; previous revision: 1.11
done
Checking in themes/qute/global/tabbox.css;
/cvsroot/mozilla/toolkit/themes/qute/global/tabbox.css,v  <--  tabbox.css
new revision: 1.4; previous revision: 1.3
done
Checking in themes/winstripe/global/tabbox.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/tabbox.css,v  <--  tabbox.css
new revision: 1.4; previous revision: 1.3
done

Comment 5

[Aaron Leventhal]

Rechecked in, leaving open for Seamonkey work.

Checking in content/widgets/tabbox.xml;
/cvsroot/mozilla/toolkit/content/widgets/tabbox.xml,v  <--  tabbox.xml
new revision: 1.14; previous revision: 1.13
done
Checking in themes/qute/global/tabbox.css;
/cvsroot/mozilla/toolkit/themes/qute/global/tabbox.css,v  <--  tabbox.css
new revision: 1.6; previous revision: 1.5
done
Checking in themes/winstripe/global/tabbox.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/tabbox.css,v  <--  tabbox.css
new revision: 1.6; previous revision: 1.5
done

Comment 6

[Aaron Leventhal]

Attachment: Same fix for Seamonkey

Comment 7

[[e-mail]]

Comment on attachment 153423 [details] [diff] [review]
Same fix for Seamonkey

You need class="box-inherit" and various xbl:inherits in case someone wants to
e.g. swap the text and icon. See button.xml for example. Have you tried Venkman
with this patch? I think you might need to adjust the display of its little
tabs. You should also try to use a unique class rather than a generic tag for
your style rules. Can you explain why you might possibly need that min-width?
Finally r+sr is a layout convenience, it does not apply to xpfe, until jag et
al decide that it does.

Comment 8

[Stephen Walker]

http://lxr.mozilla.org/seamonkey/source/toolkit/themes/winstripe/global/browser.css#33
needs to be patched to work with the added hbox.

Comment 9

[Aaron Leventhal]

Comment on attachment 153423 [details] [diff] [review]
Same fix for Seamonkey

> You need class="box-inherit" and various xbl:inherits in case someone wants
> to e.g. swap the text and icon.
Done.

> Have you tried Venkman with this patch? 
The tabs look fine in Venkman, although they are not focusable.

> You should also try to use a unique class rather than a generic tag for
> your style rules. 
Done

> Can you explain why you might possibly need that min-width?
Otherwise the tabs ended up very wide. I looked around in other parts of
tabbrowser.xml and saw similar min-width lines. What's a better way?

Comment 10

[Stephen Walker]

Comment on attachment 154022 [details] [diff] [review]
Addresses some of Neil's comments. Questions on others.

Should't the tabbox rules that were added earlier be using .tab-middle instead
of hbox?  The classic/global/win/tabbox.css change should be added/updated as
well.

I'm assuming you meant the toolkit change to be added to the tab instead of
tabs-closebutton.

Comment 11

[Aaron Leventhal]

Attachment: Updated patch address swalker's comments

Comment 12

[Aaron Leventhal]

Attachment: Removes min-width

min-width wasn't necessary after all, but flex="1" is necessary.

Comment 13

[Stephen Walker]

Comment on attachment 154094 [details] [diff] [review]
Removes min-width

r=me, minus the tabbrowser.xml changes which look to be for bug 175893, if the
tab:focus rules lose the unnecessary > hbox.

Comment 14

[Aaron Leventhal]

Attachment: Patch with sdwalker's corrections

Comment 15

[Aaron Leventhal]

Comment on attachment 154221 [details] [diff] [review]
Patch with sdwalker's corrections

Carrying sdwalker's r= forward

Comment 16

[[e-mail]]

Attachment: Linux screenshot

Comment 17

[[e-mail]]

Comment on attachment 154221 [details] [diff] [review]
Patch with sdwalker's corrections

sr=me if you ensure that someone has filed a bug to make linux tabs less ugly
in general.

Comment 18

[Aaron Leventhal]

Checking in toolkit/content/widgets/tabbox.xml;
/cvsroot/mozilla/toolkit/content/widgets/tabbox.xml,v  <--  tabbox.xml
new revision: 1.15; previous revision: 1.14
done
Checking in xpfe/global/resources/content/bindings/tabbox.xml;
/cvsroot/mozilla/xpfe/global/resources/content/bindings/tabbox.xml,v  <-- 
tabbox.xml
new revision: 1.32; previous revision: 1.31
done
Checking in themes/classic/global/win/tabbox.css;
/cvsroot/mozilla/themes/classic/global/win/tabbox.css,v  <--  tabbox.css
new revision: 1.13; previous revision: 1.12
done
Checking in toolkit/themes/qute/global/browser.css;
/cvsroot/mozilla/toolkit/themes/qute/global/browser.css,v  <--  browser.css
new revision: 1.3; previous revision: 1.2
done
Checking in toolkit/themes/qute/global/tabbox.css;
/cvsroot/mozilla/toolkit/themes/qute/global/tabbox.css,v  <--  tabbox.css
new revision: 1.7; previous revision: 1.6
done
Checking in toolkit/themes/winstripe/global/browser.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/browser.css,v  <--  browser.css
new revision: 1.3; previous revision: 1.2
done
Checking in toolkit/themes/winstripe/global/tabbox.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/tabbox.css,v  <--  tabbox.css
new revision: 1.7; previous revision: 1.6
done