1333168 - Bold text in permission notifications is breaking lines unnecessarily

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Description

[Andrew Swan [:aswan]]

Attachment: Screen Shot 2017-01-23 at 11.50.10 AM.png

See the attached screenshot.  There are meant to be two lines of text, each with a mix of normal and bold text.  But the bold text is causing line breaks.
The mixed text is implemented as a <description> with an embedded <label>, the <label> has a class that just sets font-weight.  I tried adding display: inline to the label but that doesn't seem to make any difference.  I suspect there's some simple CSS tweak here but I don't know what it is...

Comment 2

[Florian Quèze [:florian]]

The problem happens only when the add-on name is too long to fit on a single line.

To fix it, change the "label" to a "span", and add "display: inline" in its CSS.

Comment 3

[Andrew Swan [:aswan]]

I could swear I started out this way and was told not to mix html and xul.  But I'll give it a shot.

Comment 4

[Florian Quèze [:florian]]

(In reply to Andrew Swan [:aswan] from comment #3)
> I could swear I started out this way and was told not to mix html and xul.

Yes, I told you to not mix html and xul unless there was a reason to do it. This bug is the reason. Sorry :-(.

Comment 5

[Andrew Swan [:aswan]]

Attachment: Bug 1333168 Use <span> for addon name in permission dialog

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

Comment 6

[Florian Quèze [:florian]]

Comment on attachment 8832756 [details]
Bug 1333168 Use <span> for addon name in permission dialog

https://reviewboard.mozilla.org/r/108978/#review110290

::: browser/modules/ExtensionsUI.jsm:177
(Diff revision 1)
>      if (name.length > 50) {
>        name = name.slice(0, 49) + "…";
>      }
>      name = this._sanitizeName(name);
>  
> -    let addonLabel = `<label class="addon-webext-name">${name}</label>`;
> +    let addonLabel = `<span class="addon-webext-name">${name}</span>`;

You need to make this change in showInstallNotification too, right?

Now that it's no longer a label, should we rename this variable to eg. addonName?

Comment 7

[Andrew Swan [:aswan]]

Comment on attachment 8832756 [details]
Bug 1333168 Use <span> for addon name in permission dialog

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/108978/diff/1-2/

Comment 8

[Andrew Swan [:aswan]]

Comment on attachment 8832756 [details]
Bug 1333168 Use <span> for addon name in permission dialog

https://reviewboard.mozilla.org/r/108978/#review110290

> You need to make this change in showInstallNotification too, right?
> 
> Now that it's no longer a label, should we rename this variable to eg. addonName?

Whoops, thanks, that's what I get for pushing late at night...

Comment 9

[Florian Quèze [:florian]]

Comment on attachment 8832756 [details]
Bug 1333168 Use <span> for addon name in permission dialog

https://reviewboard.mozilla.org/r/108978/#review110378

Looks good (note: I haven't tested the patch locally, but I had tested with devtools the suggestion I proposed in comment 2, so I'm reasonably confident).

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c1fed3356757
Use <span> for addon name in permission dialog r=florian

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/c1fed3356757

Comment 12

[Vasilica Tamas, QA [:vtamas]]

Verified fixed on Firefox 54.0a1 (2017-02-20/21) under Windows 10 64-bit, Mac OS X 10.12.1 and Ubuntu 16.04 32-bit. Add-ons name are correctly displayed and there are no longer unnecessary line breaks.