Closed
Bug 1140592
Opened 10 years ago
Closed 10 years ago
about:passwords header height does not match other about: pages
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox40 fixed)
RESOLVED
FIXED
Firefox 40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: liuche, Assigned: u535116, Mentored)
References
Details
(Whiteboard: [good first bug][lang=js])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
liuche
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Comment 1•10 years ago
|
||
Do you know where the CSS for this lives? A source link would definitely make this a good first bug.
Reporter | ||
Updated•10 years ago
|
Mentor: liuche
Reporter | ||
Updated•10 years ago
|
Whiteboard: [good first bug] → [good first bug][lang=js]
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•10 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•10 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=
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!
Adjusted the icon size, which was forcing about:passwords header to have a greater height than the about: pages.
Attachment #8585518 -
Flags: review+
Please disregard previous patch, had issues as a new hg user :-)
Attachment #8585521 -
Flags: review+
Reporter | ||
Updated•10 years ago
|
Attachment #8585518 -
Attachment is obsolete: true
Attachment #8585518 -
Flags: review+
Reporter | ||
Comment 8•10 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•10 years ago
|
Assignee: nobody → mking
Thanks for the tip, I'll remember the reviewer flag for next time. Thanks!
Reporter | ||
Comment 10•10 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•10 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•10 years ago
|
||
Target Milestone: --- → Firefox 40
Comment 13•10 years ago
|
||
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•