[Form Autofill] "Saved credit cards…" buttons is too tall if associated checkbox wraps on two lines

VERIFIED FIXED in Firefox 59
(Needinfo from 2 people)

Status

()

defect
P3
normal
VERIFIED FIXED
2 years ago
8 months ago

People

(Reporter: flod, Assigned: scottwu, NeedInfo)

Tracking

(Blocks 2 bugs)

Trunk
mozilla59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 unaffected, firefox59 fixed, firefox66 verified)

Details

(Whiteboard: [form autofill:V2])

Attachments

(5 attachments)

Posted image screenshot.png
See attached screenshot: "Autofill credit cards" is much longer in Italian, and wraps on two lines. The button height seems to be affected by that.
Got also an issue from French: the learn more link should be inline with the label.
Whiteboard: [form autofill:V2]
Assignee: nobody → scwwu
Status: NEW → ASSIGNED
Component: Form Manager → Form Autofill
Form Autofill is only enabled on Fx58 en-US build so set firefox58:unaffected.
Comment on attachment 8941746 [details]
Bug 1412247 - Ensure checkbox label and "Learn more" link stay on the same line.

https://reviewboard.mozilla.org/r/211966/#review218072

::: browser/extensions/formautofill/FormAutofillUtils.jsm:169
(Diff revision 1)
>  };
>  
>  this.FormAutofillUtils = {
>    get AUTOFILL_FIELDS_THRESHOLD() { return 3; },
>    get isAutofillEnabled() { return this.isAutofillAddressesEnabled || this.isAutofillCreditCardsEnabled; },
> -  get isAutofillCreditCardsEnabled() { return this.isAutofillCreditCardsAvailable && this._isAutofillCreditCardsEnabled; },
> +  get isAutofillCreditCardsEnabled() { return this.isAutofillCreditCardsAvailable && this.isAutofillCreditCardsEnabled; },

You introduced a recursive call here. Can't we use the original `isAutofillCreditCardsEnabled` directly?
Attachment #8941746 - Flags: review?(lchang)
Comment on attachment 8941746 [details]
Bug 1412247 - Ensure checkbox label and "Learn more" link stay on the same line.

https://reviewboard.mozilla.org/r/211966/#review218072

> You introduced a recursive call here. Can't we use the original `isAutofillCreditCardsEnabled` directly?

Oops that's embarrassing.. I originally thought I added the underscore by mistake and wanted to correct it. I've changed it back.
Comment on attachment 8941746 [details]
Bug 1412247 - Ensure checkbox label and "Learn more" link stay on the same line.

https://reviewboard.mozilla.org/r/211966/#review218460

Thanks.
Attachment #8941746 - Flags: review?(lchang) → review+
Keywords: checkin-needed
Pushed by lchang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/48d3e471b9aa
Ensure checkbox label and "Learn more" link stay on the same line. r=lchang
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/48d3e471b9aa
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Depends on: 1420884
Depends on: 1432052
I managed to reproduced this issue using Firefox 58.0a1 Italian-build (2017.10.27) on Windows 8.1 x64.
I verified on 2018.01.15 Fx 59.0a1 (build ID=20180115100056) Italian-build and I observed that the checkbox lable and the "Learn more" link is not in the same line, attached 2018.01.15-good.jpg. 
Then I verified on 2018.01.16 build (ID=20180116100114)and attached for this 2018.01.16-bad.jpg, the same happend on Fx 60.0a1 it-build from 2018.02.02.
Flags: needinfo?(scottcwwu)
Flags: needinfo?(vchen)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Depends on: 1488011
This bug should remain resolved-fixed as we are not backing this patch out and we are not landing any more patches in this bug.
Status: REOPENED → RESOLVED
Closed: 2 years ago10 months ago
Resolution: --- → FIXED
I can Confirm this issue as Fixed in Firefox 66.0a1 (2018-12-20).
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.