1140592 - about:passwords header height does not match other about: pages

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Comment 1

[Mike Hoye [:mhoye]]

Do you know where the CSS for this lives? A source link would definitely make this a good first bug.

Comment 2

[u535116]

Hello,

I'd like to take this bug. This would be my first time working on a Mozilla project, but I am familiar with Android development. Would love to contribute. Thanks!

Comment 3

[Chenxia Liu [:liuche]]

Thanks Matt, do you have a build set up? If not, take a look at the setup wiki: https://wiki.mozilla.org/Mobile/Fennec/Android

(Don't be intimidated, only the first few steps are really necessary!)

If you run into problems, you can ping in #mobile, or needinfo me (check the "Need more information" box).

Just a note, this will be work with the JS part of Firefox for Android, so there isn't any Java involved.

Comment 4

[Chenxia Liu [:liuche]]

The relevant code is in aboutPasswords.css and aboutPasswords.xhtml (including some other relevant code too):

http://mxr.mozilla.org/mozilla-central/find?string=aboutpasswords&tree=mozilla-central&hint=

Comment 5

[u535116]

Great! I'll get set up and let you know if I have any questions.

I'm familiar with JavaScript and CSS as well so this should be a nice introduction.

Thanks!

Comment 6

[u535116]

Attachment: about_passwords_height.patch

Adjusted the icon size, which was forcing about:passwords header to have a greater height than the about: pages.

Comment 7

[u535116]

Attachment: about_passwords_height_fix.patch

Please disregard previous patch, had issues as a new hg user :-)

Comment 8

[Chenxia Liu [:liuche]]

Comment on attachment 8585521 [details] [diff] [review]
about_passwords_height_fix.patch

Thanks for the patch, Matt! I'll get to this today or tomorrow.

Just a note, to request review you set the r? flag and set the value to your reviewer (usually the autocomplete service will match the format of :<reviewer_irc>), and your reviewer will toggle the flag to r+ when they think it's good to land in the main code base.

Comment 9

[u535116]

Thanks for the tip, I'll remember the reviewer flag for next time. Thanks!

Comment 10

[Chenxia Liu [:liuche]]

Comment on attachment 8585521 [details] [diff] [review]
about_passwords_height_fix.patch

Review of attachment 8585521 [details] [diff] [review]:
-----------------------------------------------------------------

This looks much better, thanks for the patch! I'll land it after addressing comments.

::: mobile/android/themes/core/aboutPasswords.css
@@ +45,2 @@
>    background-repeat: no-repeat;
> +  height: 20px;

Why not make this match the background-size too?

Comment 11

[u535116]

If the background-size was any smaller, the icon looked a bit too small. Adjusting the width/height of the element and having a slightly larger background-size kept a reasonably sized icon while fixing the header height problem.

Comment 12

[Chenxia Liu [:liuche]]

https://hg.mozilla.org/integration/fx-team/rev/45af6b8fe218

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/45af6b8fe218