1393001 - [Form Autofill] Edit and Add forms have the wrong UI compared to spec

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect)

Description

[Bogdan Maris, Desktop Test Engineering]

[Affected versions]:
- Firefox 56.0b4 (trybuild)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=04cc6bb1dac3&selectedJob=124798144
- latest Nightly 57.0a1

[Affected platforms]:
- Windows 10 64bit
- macOS 10.12.6
- Ubuntu 16.04 64bit

[Steps to reproduce]:
1. Start Firefox
2. Open about:preferences#privacy
3. Click 'Saved Addresses'
4. Click Add... and Edit...

[Expected result]:
- The form is just like in the specs https://mozilla.invisionapp.com/share/MQ8J8K4CE#/screens/243501869

[Actual result]:
- 56.0b4 the forms are close to the specs, they lack the ellipses at the end of each field name, and two unexpected scrollbars can be seen.
- 57.0b1 the forms are totally misaligned, misplaced, no ellipses but the scrollbars are no longer there.

[Regression range]:
- This is a recent regression. The form looked good in Nightly from 2017-07-13 (even though the ellipses were not implemented there as well).

It's quite difficult to track for a regression because I can see two of them (the UI changed two times). Take note that Nightly from 2017-07-13 was closest to specs as stated above.

1. From 2017-08-07 I see that the input areas have different lengths. Until 2017-08-15
- here is the pushlog for the first regression:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=47248637eafa9a38dade8dc3aa6c4736177c8d8d&tochange=fde1450a4368d04e97174e2eb00fb48901179857

- Screenshot of the issue: http://i.imgur.com/xRxMJtZ.png

2. From 2017-08-16 I see that the space between input areas nearly 0. Until 2017-08-22 (latest Nightly)
- here is the pushlog for the second regression:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=b131a336d02f715d73724704cc54517b4929d33a&tochange=76c117fe7e1436e44dd91ce0b2da58cf3cd2a564 

- Screenshot of the issue: http://i.imgur.com/Qtd1M4g.png

[Additional notes]:
- Here are some screenshots of the actual form UI (I used imgur for posting images because there were three of them and I did not want to spam bugzilla with comments):
-- Firefox 56.0b4 (trybuild) - http://i.imgur.com/UaTMmWm.png
-- latest Nightly 57.0a1 - http://i.imgur.com/Qtd1M4g.png
-- Nightly between 2017-08-07 - 2017-08-15 - http://i.imgur.com/xRxMJtZ.png

Comment 1

[Luke Chang [:lchang] (inactive)]

NI Scott for follow-up.

Comment 2

[Scott Wu [:scottwu]]

Attachment: Bug 1393001 - Fix edit address dialog form layout problems.

Review commit: https://reviewboard.mozilla.org/r/172610/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/172610/

Comment 3

[Scott Wu [:scottwu]]

Attachment: Layout fixed

The patch fixes the layout issues. Some CSS rules were changed to hopefully make it more robust. The dialog inherits common styles, which were changed by photon, thus causing the issues. Thanks for the detailed bug report!

Comment 5

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8901131 [details]
Bug 1393001 - Fix edit address dialog form layout problems.

https://reviewboard.mozilla.org/r/172610/#review178134

Hasn't "skin/shared/editAddress.css" been renamed? You need to rebase on it. Overall it looks good. Thanks.

Comment 6

[Scott Wu [:scottwu]]

Comment on attachment 8901131 [details]
Bug 1393001 - Fix edit address dialog form layout problems.

https://reviewboard.mozilla.org/r/172610/#review178134

The rename already happened before this patch. I've just rebased to central again and it runs fine. Thanks!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/19142556f7cb
Fix edit address dialog form layout problems. r=lchang

Comment 8

[Scott Wu [:scottwu]]

Comment on attachment 8901131 [details]
Bug 1393001 - Fix edit address dialog form layout problems.

Approval Request Comment
[Feature/Bug causing the regression]:
The edit address dialog includes stylesheets that were modified by another bug (Bug 1377174).

[User impact if declined]:
Users will see a broken looking form that are not aligned and with margins missing.

[Is this code covered by automated tests?]:
No. We do not have automated tests for page layout.

[Has the fix been verified in Nightly?]:
Yes I can verify it's been fixed on Nightly.(https://bug1393001.bmoattachments.org/attachment.cgi?id=8901135)

[Needs manual test from QE? If yes, steps to reproduce]: 
Yes.
1. Open Firefox Nightly
2. Open Preferences
3. Click "Privacy & Security"
4. Click "Saved Addresses" under "Forms & Passwords"
5. Click "Add"
6. Verify that the layout is the same as the visual spec: (https://mozilla.invisionapp.com/share/MQ8J8K4CE#/screens/243501867)

[List of other uplifts needed for the feature/fix]:
None.

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
It only modifies CSS rules.

[String changes made/needed]:
None.

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/19142556f7cb

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8901131 [details]
Bug 1393001 - Fix edit address dialog form layout problems.

CSS fix for form autofill, let's uplift for 56 beta 7.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Needs a rebased patch for Beta uplift.

Comment 12

[Scott Wu [:scottwu]]

I just found that the bug doesn't affect beta. Bug 1377174 is the cause of this regression, but it doesn't land until 57, so uplift is not needed. Sorry for the mix-up! But I still hope QA could verify that beta is not affected by this bug. Thanks.

Comment 13

[Bogdan Maris, Desktop Test Engineering]

I confirm that this is fixed in 56beta8 across platforms (macOS 10.12.6, Ubuntu 16.04 64bit and Windows 10 64bit). The only thing that's not exactly as the spec are the colons that are missing from latest builds but are present in specs.

(eg: "First Name:" vs "First Name")

Will the colons still be implemented?

Comment 14

[Scott Wu [:scottwu]]

I have confirmed with UX and visual that the colons will not be implemented, because we could not find its use elsewhere in Preferences. Thanks!