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

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect)

Description

[Bogdan Maris, Desktop QA]

Attachment: 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

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Yikes. Hey ntim, got any idea how I might have broken things here?

Comment 2

[Tim Nguyen :ntim]

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 ?).

Comment 3

[Mike Conley (:mconley) (:⚙️)]

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?

Comment 4

[Matthew N. [:MattN]]

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.

Comment 5

[:Gijs (he/him)]

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.

Comment 6

[Matthew N. [:MattN]]

(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`.

Comment 7

[Matthew N. [:MattN]]

Attachment: 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/

Comment 8

[Matthew N. [:MattN]]

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/

Comment 9

[Matthew N. [:MattN]]

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…

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Matt, are you still working on this? Thanks

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: 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/

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Try push with MOZ_SCREENSHOTS enabled,
https://treeherder.mozilla.org/#/jobs?repo=try&revision=365bcf31d03d

Comment 13

[:Gijs (he/him)]

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. :-(

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

Yeah, tested locally on Windows 10.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
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

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4d012e675ac3

Comment 17

[Rezaul Huque Nayeem]

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

Comment 18

[Hossain Al Ikram [:ikram] (QA Contact)]

Sorry, Done it by mistake, Please ignore as spam!