Edit email or password field inside Save Logins is larger then it should

VERIFIED FIXED in Firefox 50

Status

()

Firefox
Preferences
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: bogdan_maris, Assigned: jaws)

Tracking

({regression})

Trunk
Firefox 50
regression
Points:
---

Firefox Tracking Flags

(firefox43 wontfix, firefox44 wontfix, firefox45 wontfix, firefox46 wontfix, firefox47 wontfix, firefox48 fix-optional, firefox49 fix-optional, firefox50 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

2 years ago
Created attachment 8708911 [details]
Screenshot showing the issue

Affected builds:
- Firefox 44 beta 9
- latest Developer Edition 45.0a2
- latest Nightly 46.0a1

Affected OS's:
- Windows 7 64-bit
- Windows 10 64-bit
- Mac OS X 10.11
- Ubuntu 14.04 64-bit

STR:
1. Open Firefox
2. Visit any webpage that has login
3. Login and remember password
4. Go to about:preferences#security
5. Click 'Saved Logins...'
6. Double click email or password (after hitting show passwords).

Notes:
- Image from above (Broken) is from Firefox 44 beta 9, image from bellow (Good) is from 43.0RC
- This is a regression:

m-c

Last good revision: 3edc8d4a1e198314f5d7e
bd2967b85842beef602 (2015-10-06)
First bad revision: d1c5a7c5b4331ee9ea544
3de893fcfd0a5b80e2a (2015-10-07)

Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3edc8d4a1e198314f5d7ebd2967b85842beef602&tochange=d1c5a7c5b4331ee9ea5443de893fcfd0a5b80e2a

m-i

Last good revision: d371c97b84b58dc87e539
869fca437387c307fca
First bad revision: d37602b95f4445605a45b
e788305b00865666264
Pushlog:

https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=d371c97b84b58dc87e539869fca437387c307fca&tochange=d37602b95f4445605a45be788305b00865666264

Culprit bug based on mozregression:
Bug 1110511 - Implement about:tabcrashed spec
(Reporter)

Updated

2 years ago
Flags: needinfo?(mconley)
Yikes. Hey ntim, got any idea how I might have broken things here?
Flags: needinfo?(mconley) → needinfo?(ntim.bugs)

Comment 2

2 years ago
Adding the min-height seems to have regressed this: https://hg.mozilla.org/integration/fx-team/rev/717c687684d3#l8.31 . 

I originally suggested change for better HDPI support, but didn't think about the smaller inputs. Reverting the change will surely fix this bug, but there's surely a better fix (probably overriding the min-height specially for the table ?).
Flags: needinfo?(ntim.bugs)
Note that if I remove the min-height, things don't look amazing either:

http://i.imgur.com/J7MpgDU.png

Hey MattN, do you remember who the UX contact was for the password manager improvements?
Flags: needinfo?(MattN+bmo)
It's Ryan Feeley but this is actually a preferences stylesheet issue. The standalone passwordManager.xul window likely doesn't have this problem, it's only when the additional preferences stylesheets are injected I think.
Component: General → Preferences
Flags: needinfo?(MattN+bmo)

Comment 5

2 years ago
See also bug 1247214. Seems we're using 1.3em as a font-size for the content of any textbox in a subdialog (dialog.css) on OS X, and 1.2em for Windows:


label,
textbox,
description,
.tab-text,
caption > label {
  font-size: 1.3em;
}

( https://dxr.mozilla.org/mozilla-central/source/browser/themes/windows/preferences/in-content/dialog.css , https://dxr.mozilla.org/mozilla-central/source/browser/themes/osx/preferences/in-content/dialog.css )

We should avoid that applying to this textbox (where does that textbox come from, btw? It's hard to inspect it because it goes away as soon as it loses focus...) and to context menus.
See Also: → bug 1247214
status-firefox43: unaffected → wontfix
status-firefox44: affected → wontfix
status-firefox45: affected → wontfix
status-firefox46: affected → wontfix
status-firefox47: --- → wontfix
status-firefox48: --- → affected
(In reply to :Gijs Kruitbosch from comment #5)
> We should avoid that applying to this textbox (where does that textbox come
> from, btw? It's hard to inspect it because it goes away as soon as it loses
> focus...) and to context menus.

It's `textbox.tree-input` which conveniently doesn't disappear from the document when it's not visible so you can always see it in devtools.

Do we want to increase the specificity of the textbox rule to exclude this though? That may cause other problems.

Btw. in-content prefs top-level panes and advanced panes are all covered in mozscreenshots now. No subdialogs are currently covered.

I'll try add `:not(.tree-input)` to `textbox`.
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Created attachment 8733079 [details]
MozReview Request: Bug 1240439 - WIP: Preferences: Don't increase the font size of textboxes in trees.

Also fix incorrect license header.

Review commit: https://reviewboard.mozilla.org/r/41533/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/41533/
Attachment #8733079 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8733079 [details]
MozReview Request: Bug 1240439 - WIP: Preferences: Don't increase the font size of textboxes in trees.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41533/diff/1-2/
Attachment #8733079 - Attachment description: MozReview Request: Bug 1240439 - Preferences: Don't increase the font size of textboxes in trees. r=Gijs → MozReview Request: Bug 1240439 - WIP: Preferences: Don't increase the font size of textboxes in trees.
Comment on attachment 8733079 [details]
MozReview Request: Bug 1240439 - WIP: Preferences: Don't increase the font size of textboxes in trees.

As expected, changing the specificity breaks stuff…
Attachment #8733079 - Flags: review?(gijskruitbosch+bugs)
Matt, are you still working on this? Thanks
status-firefox48: affected → fix-optional
status-firefox49: --- → fix-optional
status-firefox50: --- → fix-optional
Flags: needinfo?(MattN+bmo)
Created attachment 8769444 [details]
Bug 1240439 - Remove padding, font-size, and min-height rules from being applied to tree-inputs so they will fit nicely in to their tree row.

Review commit: https://reviewboard.mozilla.org/r/63350/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/63350/
Attachment #8769444 - Flags: review?(gijskruitbosch+bugs)
Attachment #8733079 - Attachment is obsolete: true
Try push with MOZ_SCREENSHOTS enabled,
https://treeherder.mozilla.org/#/jobs?repo=try&revision=365bcf31d03d
Assignee: MattN+bmo → jaws
Flags: needinfo?(MattN+bmo)

Updated

2 years ago
Attachment #8769444 - Flags: review?(gijskruitbosch+bugs) → review+

Comment 13

2 years ago
Comment on attachment 8769444 [details]
Bug 1240439 - Remove padding, font-size, and min-height rules from being applied to tree-inputs so they will fit nicely in to their tree row.

https://reviewboard.mozilla.org/r/63350/#review60314

This looks OK to me from a code perspective. I'm assuming you tested manually on at least 1 platform - though I don't think we have all that many things that would match `textbox.tree-input` in-content anyway, so it's likely fine. Note that the mozscreenshots pushes seem to have not worked. :-(
Yeah, tested locally on Windows 10.
Keywords: checkin-needed

Comment 15

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/4d012e675ac3
Remove padding, font-size, and min-height rules from being applied to tree-inputs so they will fit nicely in to their tree row. r=gijs
Keywords: checkin-needed

Comment 16

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4d012e675ac3
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: fix-optional → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50

Comment 17

2 years ago
Reproduced this issue in firefox nightly 48.0a1 (2016-04-08) with ubuntu 16.04 (64 bit)

Verified as this issue fixed with latest firefox nightly 50.0a1 (Build ID: 20160726030213)
Mozilla/5.0 (X11; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0
QA Whiteboard: [bugday-20160727]
Status: RESOLVED → VERIFIED
status-firefox50: fixed → verified
Flags: needinfo?(marufislam482)
Sorry, Done it by mistake, Please ignore as spam!
Flags: needinfo?(marufislam482)
You need to log in before you can comment on or make changes to this bug.