[Affected versions]: Firefox 56.0a1 (2017-06-15) Firefox 55.0b2 (20170615063713) [Affected platforms]: Windows 10 64-bit Ubuntu 16.04 32-bit [Steps to reproduce]: 1.Launch Firefox with a clean profile. 2.Navigate to https://addons.allizom.org/en-US/firefox/addon/little-flowers/ 3.Click on “+ Add to Firefox” green button. [Expected Results]: Theme name is bolded. [Actual Results]: - Theme name is not not bolded just like in installation confirmation doorhanger: https://www.screencast.com/t/jJKVmScbsQ - See attached screenshot.

I can't get this to occur on addons.mozilla.org production site, so dropping priority based on that.

It looks bold in the screenshot and I see it bolded from addons.mozilla.org on Nightly. Am I missing something? This looks invalid to me.

Try it on addons.allizom.org, the link in comment 0.

Ah, there were two screenshots. Oops. So it's the prompt to give permissions to install add-ons.

Reassigning since there hasn't been any recent activity.

There were some changes in bug 1369482 recently that should make this easier to implement. Take not that in its current state it would introduce an unwanted space before the question mark, however.

Comment on attachment 8945024 [details] Bug 1373642: Theme name is not bolded in installation pop-up - Fixed https://reviewboard.mozilla.org/r/215226/#review220774 Static analysis found 2 defects in this patch. - 2 defects found by mozlint You can run this analysis locally with: - `./mach lint check path/to/file` (Python/Javascript/wpt) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: browser/base/content/test/general/browser_bug553455.js:781 (Diff revision 1) > is(installs.length, 1, "Should be one pending install"); > installs[0].cancel(); > > Services.perms.remove(makeURI("http://example.com/"), "install"); > await removeTab(); > + Assert.equals(document.getElementById("addon-webext-name"), null) Error: Missing semicolon. [eslint: semi] ::: browser/base/content/test/general/browser_bug553455.js:822 (Diff revision 1) > AddonManager.getAddonByID("theme-xpi@tests.mozilla.org", function(result) { > resolve(result); > }); > }); > isnot(addon, null, "Test theme will have been installed"); > - addon.uninstall(); > + addon.uninstall();//addon-webext-perm-header Error: Expected space or tab after '//' in comment. [eslint: spaced-comment]

Comment on attachment 8945024 [details] Bug 1373642: Theme name is not bolded in installation pop-up - Fixed https://reviewboard.mozilla.org/r/215226/#review220782 Static analysis found 2 defects in this patch. - 2 defects found by mozlint You can run this analysis locally with: - `./mach lint check path/to/file` (Python/Javascript/wpt) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: browser/base/content/test/general/browser_bug553455.js:781 (Diff revision 1) > is(installs.length, 1, "Should be one pending install"); > installs[0].cancel(); > > Services.perms.remove(makeURI("http://example.com/"), "install"); > await removeTab(); > + Assert.equals(document.getElementById("addon-webext-name"), null) Error: Missing semicolon. [eslint: semi] ::: browser/base/content/test/general/browser_bug553455.js:822 (Diff revision 1) > AddonManager.getAddonByID("theme-xpi@tests.mozilla.org", function(result) { > resolve(result); > }); > }); > isnot(addon, null, "Test theme will have been installed"); > - addon.uninstall(); > + addon.uninstall();//addon-webext-perm-header Error: Expected space or tab after '//' in comment. [eslint: spaced-comment]

Comment on attachment 8945024 [details] Bug 1373642: Theme name is not bolded in installation pop-up - Fixed https://reviewboard.mozilla.org/r/215226/#review220792 Nice, thanks! ::: browser/base/content/browser-addons.js:648 (Diff revision 1) > this._install(data, notify); > return; > } > > let strings = { > - header: gNavigatorBundle.getFormattedString("webextPerms.header", [data.name]), > + header: gNavigatorBundle.getFormattedString("webextPerms.header", [`<span class="addon-webext-name">${data.name}</span>`]), Can you move `<span class="addon-webext-name">${data.name}</span>` in a separate addonName variable like it's done in other places ? https://dxr.mozilla.org/mozilla-central/source/browser/modules/ExtensionsUI.jsm#252 ::: browser/base/content/test/general/browser_bug380960.js (Diff revision 1) > tab = BrowserTestUtils.addTab(gBrowser, "about:blank", { skipAnimation: true }); > gBrowser.removeTab(tab, { animate: true }); > gBrowser.removeTab(tab); > is(tab.parentNode, null, "tab removed immediately when calling removeTab again after the animation was kicked off"); > } > - nit: please undo this change ::: browser/base/content/test/general/browser_bug553455.js:781 (Diff revision 1) > is(installs.length, 1, "Should be one pending install"); > installs[0].cancel(); > > Services.perms.remove(makeURI("http://example.com/"), "install"); > await removeTab(); > + Assert.equals(document.getElementById("addon-webext-name"), null) Use `ok` instead of Assert.equals ok(document.getElementById("addon-webext-name"), "Add-on name should be wrapped in a bold label"); ::: browser/base/content/test/general/browser_bug553455.js:822 (Diff revision 1) > AddonManager.getAddonByID("theme-xpi@tests.mozilla.org", function(result) { > resolve(result); > }); > }); > isnot(addon, null, "Test theme will have been installed"); > - addon.uninstall(); > + addon.uninstall();//addon-webext-perm-header Why did you add this comment ? ::: browser/locales/en-US/chrome/browser/browser.properties:37 (Diff revision 1) > xpinstallDisabledMessage=Software installation is currently disabled. Click Enable and try again. > xpinstallDisabledButton=Enable > xpinstallDisabledButton.accesskey=n > > # LOCALIZATION NOTE (webextPerms.header) > -# This string is used as a header in the webextension permissions dialog, > +# This string is https://bug1308309.bmoattachments.org/attachment.cgi?id=8814612used as a header in the webextension permissions dialog, nit: please undo this change ::: moz.configure:17 (Diff revision 1) > # - Fennec-specific options and rules should go in > # mobile/android/moz.configure. > # - Spidermonkey-specific options and rules should go in js/moz.configure. > # - etc. > > + Please also undo this one :)

I found a good test case to add to in order to test whether or not the theme name was bolded and removed the previous assert statement completely. All other recommendations should be fixed. Thanks for the feedback!

Comment on attachment 8945024 [details] Bug 1373642: Theme name is not bolded in installation pop-up - Fixed https://reviewboard.mozilla.org/r/215226/#review220926 Static analysis found 2 defects in this patch. - 2 defects found by mozlint You can run this analysis locally with: - `./mach lint check path/to/file` (Python/Javascript/wpt) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: browser/base/content/test/general/browser_bug553455.js:781 (Diff revision 1) > is(installs.length, 1, "Should be one pending install"); > installs[0].cancel(); > > Services.perms.remove(makeURI("http://example.com/"), "install"); > await removeTab(); > + Assert.equals(document.getElementById("addon-webext-name"), null) Error: Missing semicolon. [eslint: semi] ::: browser/base/content/test/general/browser_bug553455.js:822 (Diff revision 1) > AddonManager.getAddonByID("theme-xpi@tests.mozilla.org", function(result) { > resolve(result); > }); > }); > isnot(addon, null, "Test theme will have been installed"); > - addon.uninstall(); > + addon.uninstall();//addon-webext-perm-header Error: Expected space or tab after '//' in comment. [eslint: spaced-comment]

Thanks for the patches, Connor! This is in my queue, and I'll have a response shortly. In the meantime, I'm going to have you work on bug 1418605. I'll let you know once I have feedback on this.

Comment on attachment 8945117 [details] Bug 1373642: Theme name is not bolded in installation pop-up - Review Fixes Forwarding review to mconley who is taking this one when he gets time.

Comment on attachment 8945024 [details] Bug 1373642: Theme name is not bolded in installation pop-up - Fixed Clearing review on this since mconley will be taking it.

Hm. If I'm reading this correctly, I suspect that bug 1431242 is going to independently fix this... prathiksha, can you confirm?

I've cleared the review requests until the patches are folded. I also do want to hear back from prathiksha before I review this to see if we're about to kill two birds with one stone.

Hi Mike, I'd be ok with this issue getting resolved here seeing as it's Connor's first patch but I think we should leave it off to Bug 1431242 as it uses popupnotification's built-in bolding which was put in place to fix this issue and similar ones. If Connor would like to explore more and come up with a patch that uses popupnotification's built-in bolding to fix this, I wouldn't mind excluding this part (the theme installation popup) from Bug 1431242 but at the same time, I wouldn't call that exercise a good-first-bug. :)

(In reply to :prathiksha from comment #20) > If Connor would like to explore more and come up with a patch that uses > popupnotification's built-in bolding to fix this, I wouldn't mind excluding > this part (the theme installation popup) from Bug 1431242 but at the same > time, I wouldn't call that exercise a good-first-bug. :) Okay, thanks prathiksha. There's plenty of other work we can have Connor doing, so I think we're okay here. Hey Connor, So what happened here was a minor collision - another engineer was working on a bug, which (when fixed), will also fix your bug. It's not exactly duplicate work, but it's in the ballpark. prathiksha's solution solves the bolding case for _all_ WebExtension permission dialogs, instead of spot-fixing just one of them. I think we should go that route. Sorry about that! I know it's kind of disappointing for a bug to turn out this way, but there's plenty of other bugs for you to work on, so many chances for you to get a patch landed. :) So for now, I'm going to unassign Connor from this bug, and mark it as blocked by bug 1431242.

We may as well just call this a dupe at this point. It might be worth making sure that this prompt is in the STR for QA in bug 1431242, though.