413236 - nodoka theme git: focus indicator for location bar covers too large an area

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[u294409]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; pl; rv:1.9b3pre) Gecko/2008012004 Fedora/8 (Moonshine) Minefield/3.0b3pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; pl; rv:1.9b3pre) Gecko/2008012004 Fedora/8 (Moonshine) Minefield/3.0b3pre

I shouldn't copy'n'paste summary here :> .

Reproducible: Always

Steps to Reproduce:
1. download nodoka from git (compile and install it, and set it too, ofc).
2. open firefox.
3. focus some text-entry (eg on google.com) and press [tab] to focus button.
Actual Results:  
no focus indicators or wrong (locationbar)

Expected Results:  
everything like in my stupid native-firefox dream

Comment 1

[u294409]

Attachment: bad locationbar focus

Comment 2

[u294409]

Attachment: screenshot of good focus example

Comment 3

[Teune van Steeg]

see also bug 410489 comment #9

Comment 4

[u294409]

I always can find a bug and complicate life :> .

Comment 5

[Martin Sourada]

As for not having the focus drawn, it was a nodoka bug introduced recently when
trying to fix another issue... I've fixed this just now.

As for having the bad focus in location bar, it seems you are passing wrong
parameter values to gtk_draw_shadow.

Comment 6

[Teune van Steeg]

(In reply to comment #5)
> As for not having the focus drawn, it was a nodoka bug introduced recently when
> trying to fix another issue... I've fixed this just now.

Then forget what I posted in comment #3. I thought it had something to do with setting the focus flag to a widget (setting that seemed to fix that issue).

> As for having the bad focus in location bar, it seems you are passing wrong
> parameter values to gtk_draw_shadow.

The drop down button covers part of the location bar to hide the gap created by most themes to connect the text field with the button (the gap would become visible when the location bar background is yellow on secure sites). 

I already have a couple of ideas for how to fix this, but I still need to decide (and test) what would be the best solution.

As per comment #5, I'm updating the summary.

Comment 7

[Teune van Steeg]

Attachment: patch

Move the negative margin hack to the autocomplete-security-wrapper element, to prevent the drop down button to cover (the gap) part of the text field (which causes the focus drawn outside the text field to also outline the button for a small part).

BTW, Dao, for clarity, does a review from you not suffice? You are not listed as an official reviewer AFAICT.

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 299048 [details] [diff] [review]
patch

> /* keep the URL bar content LTR */
> #autocomplete-security-wrapper {
>   direction: ltr;
>   -moz-box-align: center;
>+  /* cover the white gap between the text field and the drop down button */
>+  margin-right: -3px;
>+}
>+
>+#urlbar[chromedir="rtl"] > .autocomplete-textbox-container > #autocomplete-security-wrapper {
>+  margin-right: 0px;
>+  margin-left: -3px;
> }

use -moz-margin-end, and please move /* keep the URL bar content LTR */ above or next to |direction: ltr;|, as it doesn't apply to the entire block anymore.

Ryan is allowed to review gnomestripe patches.

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 299048 [details] [diff] [review]
patch

Ah, I see the way you use the margins is intentional, as -moz-margin-end will always be margin-right due to |direction: ltr|.

Comment 10

[Teune van Steeg]

Attachment: patch v2

Updated patch: Move the comment to inside the block, above the matching rule

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 299054 [details] [diff] [review]
patch v2

a=beltzner

Comment 12

[Reed Loden [:reed]]

Checking in browser/themes/gnomestripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/gnomestripe/browser/browser.css,v  <--  browser.css
new revision: 1.166; previous revision: 1.165
done
Checking in toolkit/themes/gnomestripe/global/autocomplete.css;
/cvsroot/mozilla/toolkit/themes/gnomestripe/global/autocomplete.css,v  <--  autocomplete.css
new revision: 1.15; previous revision: 1.14
done