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

RESOLVED FIXED in Firefox 40

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: liuche, Assigned: mking, Mentored)

Tracking

(Blocks 1 bug)

Trunk
Firefox 40
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox40 fixed)

Details

(Whiteboard: [good first bug][lang=js])

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

4 years ago
No description provided.
Do you know where the CSS for this lives? A source link would definitely make this a good first bug.
Reporter

Updated

4 years ago
Mentor: liuche
Reporter

Updated

4 years ago
Whiteboard: [good first bug] → [good first bug][lang=js]
Assignee

Comment 2

4 years ago
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!
Reporter

Comment 3

4 years ago
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.
Reporter

Comment 4

4 years ago
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=
Assignee

Comment 5

4 years ago
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!
Assignee

Comment 6

4 years ago
Posted patch about_passwords_height.patch (obsolete) — Splinter Review
Adjusted the icon size, which was forcing about:passwords header to have a greater height than the about: pages.
Attachment #8585518 - Flags: review+
Assignee

Comment 7

4 years ago
Please disregard previous patch, had issues as a new hg user :-)
Attachment #8585521 - Flags: review+
Reporter

Updated

4 years ago
Attachment #8585518 - Attachment is obsolete: true
Attachment #8585518 - Flags: review+
Reporter

Comment 8

4 years ago
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.
Attachment #8585521 - Flags: review+ → review?(liuche)
Reporter

Updated

4 years ago
Assignee: nobody → mking
Assignee

Comment 9

4 years ago
Thanks for the tip, I'll remember the reviewer flag for next time. Thanks!
Reporter

Comment 10

4 years ago
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?
Attachment #8585521 - Flags: review?(liuche) → review+
Assignee

Comment 11

4 years ago
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.
Reporter

Comment 12

4 years ago
https://hg.mozilla.org/integration/fx-team/rev/45af6b8fe218
Target Milestone: --- → Firefox 40
https://hg.mozilla.org/mozilla-central/rev/45af6b8fe218
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.