Closed Bug 1296365 Opened 8 years ago Closed 5 years ago

WebExtension popups include the page URL in the window title

Categories

(WebExtensions :: Frontend, defect, P2)

Product:
WebExtensions
Component:
Frontend
Version:
48 Branch
Platform:
x86_64
Windows 10
Type:
defect

Tracking

(firefox71 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla71
Tracking Flags:
Tracking Status
firefox71 --- fixed

People

(Reporter: oleary.gabe, Assigned: nmaier)

References

Details

(Whiteboard: [design-decision-approved]triaged)

Attachments

(6 files, 1 obsolete file)

Popup Window Extension
2.38 KB, application/x-zip-compressed
Details
Chrome Popup
4.26 KB, image/png
Details
Firefox Popup
5.45 KB, image/png
Details
v2 of my patch
963 bytes, patch
Details | Diff | Splinter Review
Screenshot of Firefox
435.65 KB, image/png
Details
Bug 1296365 - Show extension name instead of URI in popup windows r?mixedpuppy
47 bytes, text/x-phabricator-request
Details | Review 
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36

Steps to reproduce:

When I create a window with a local extension page the title of the window shows the url, and then the page title.
chrome.windows.create({ url: chrome.extension.getURL('compare.html'), type: "popup", state: 'maximized' });

compare.html include:
<html>
<head>
<title>Compare</title>
...
</head>
...
</html>



Actual results:

Visible title of window is : 'moz-extension://extension-id - Compare'


Expected results:

The expected behavior from chrome is for window only display the page title, and not the url preceding: 'Compare'
Component: Untriaged → WebExtensions
OS: Unspecified → Windows 10
Product: Firefox → Toolkit
Hardware: Unspecified → x86_64
Summary: WebExtensions Title of page includes page url (ugly) → WebExtension popups include the page URL in the window title
Status: UNCONFIRMED → NEW
Component: WebExtensions → WebExtensions: Frontend
Ever confirmed: true
Whiteboard: [design-decision-needed]
Priority: -- → P5
Whiteboard: [design-decision-needed] → [design-decision-needed]triaged
We've been trying to make it clear that when something happens, the reason for it is clear to the end user. The ability to create a popup with no URL bar means that there could be absolutely no info to the user. 

In bug 1266012 we show the add-on in the URL bar and in bug 1318144 we show the add-on in the ID. I think we should continue the trend and the page title should contain Extension: [Extension name] - Page title. Much nicer than showing moz-extension and more informative to the end user.

Just pinging dveditz to check that sounds right.
Flags: needinfo?(dveditz)
The main thing we're trying to accomplish with the URL bar for web page popups is to prevent spoofing/phishing by showing exactly where the content originated from. The URL for a web extension doesn't convey much to a user except "this is an extension" so we don't need to show that as long as the popup can't be confused with one from a web page. Your proposal sounds like it does that. If it's not explicitly window.open() it doesn't even really need a URL bar; a panel with some title text would do (plus the "Extension:" prefix), which seems to be what this bug is asking for.

bug 1318144 is not right; bug 1318114 is maybe what you meant?
Flags: needinfo?(dveditz)
> bug 1318144 is not right; bug 1318114 is maybe what you meant?

Thanks for the correction and clarity. Sounds like we are good to go with this.
Whiteboard: [design-decision-needed]triaged → [design-decision-approved]triaged
Worse than the aboce, I am using

browser.windows.create(
  {titlePreface: "New bookmark",
   type: "popup",
   url: url,
   height: 150,
   width: 375,
   left: 300,
   top: 300,
   allowScriptsToClose: true
  }
)

in my extension to create a "popup", and the title piece is totally ignored, I get a resulting window with:

"moz-extension://.... - Mozilla Firefox"

as title, which is not only unreadable and unfriendly for common human beings, but non meaningful :-(

I am assuming this can be corrected / improved as part of this bug, so adding it here rather than opening a new bug.
Let me know if you prefer I open a new one. Thank you.
The usability of popups and windows from extensions is still marginal. Comment 3 will certainly improve the situation, but I'm wondering if there is more we can do. Asking for some UX input on this one. Is there a way to display the title in a user-friendly way, but still inform the user that the window comes from an extension?
Flags: needinfo?(emanuela)
Attached file Popup Window Extension — 
Simple extension that adds a toolbar button that, when clicked, will create a small and maximized popup window with the title "Page Title". Works on Firefox and Chrome.
Attached image Chrome Popup — 
Example of an extension popup created in Chrome. Note that it only displays the window title (no indication of the source of the popup).
Attached image Firefox Popup — 
Example of an extension popup in Firefox. Note the title bar shows it is from an extension, but it isn't very meaningful. This bug would clean it up a bit, adding the "Extension" prefix followed by the name of the extension, followed by the actualy HTML title. But is that going to be too long to fit onto smaller popups?
Priority: P5 → P2
My new employer, Privowny, hits this bug and the title of our WebExtension popups becomes so ugly our users and testers complain. I would like to propose a change here, for which I have a fix in hands: display the name of the extension instead of its moz-extension URL. It's a compromise between the current situation (strong) and no prefix at all for the title (weak). I am attaching this patch now.
Attachment #8970936 - Flags: review?(mixedpuppy)
Attached patch v2 of my patch — — Splinter Review 
Sorry, attached the wrong version. This one, with non-emptyness test on extension.name is better.
Attachment #8970936 - Attachment is obsolete: true
Attachment #8970936 - Flags: review?(mixedpuppy)
Attachment #8970941 - Flags: review?(mixedpuppy)
hi dveditz, any opinion about the compromise (and corresponding fix) I proposed above?
see comment 13
Flags: needinfo?(dveditz)
IMO when its in a tab, I see no reason to have the extension name included in the title.  The security dropdown should say what it is.  We should also somehow cover the window titles, attachment 8970941 [details] [diff] [review] doesn't handle that (see xpi that mconca attached).
Shane: as far as I can tell that's how tabs behave currently: the site into box says "Extension (<name>)" next to the ugly moz-extension URL in the address field but the Title is entirely specified by the extension and contains no ugly bits. Daniel's patch is only in the case when the location bar is hidden. Although it's in a file called tabbrowser.js we wouldn't really call them "tabs".

Daniel: My preference would be for Andy's comment 3 ("Extension: [<name>]" as a prefix, though probably better to match the site identity box for consistency and l10n reuse: "Extension (<name>)"). Since Web Extensions are not necessarily reviewed this is a slight bit of protection against a rogue extension trying to spoof browser UI. Then again a name that was spoofy in some panel context would probably look pretty odd on the installation prompt to start with. If we can get buy-in from other security folks I could live with your patch as-is.
Flags: needinfo?(dveditz) → needinfo?(ptheriault)
> If we can get buy-in from other security folks I could live
> with your patch as-is.

You logic sounds fine to me - Extension <name> seems like a decent compromise.
Flags: needinfo?(ptheriault)
(In reply to Daniel Veditz [:dveditz] from comment #16)

> Daniel: My preference would be for Andy's comment 3 ("Extension: [<name>]"
> as a prefix, though probably better to match the site identity box for
> consistency and l10n reuse: "Extension (<name>)"). Since Web Extensions are
> not necessarily reviewed this is a slight bit of protection against a rogue
> extension trying to spoof browser UI. Then again a name that was spoofy in
> some panel context would probably look pretty odd on the installation prompt
> to start with. If we can get buy-in from other security folks I could live
> with your patch as-is.

Thanks! I can of course rather tweak the patch to add that localized "Extension (*)" string. Let me know if I should spend two cycles on that?
Comment on attachment 8970941 [details] [diff] [review]
v2 of my patch

>diff --git a/browser/base/content/tabbrowser.js b/browser/base/content/tabbrowser.js
>index 784986177f05..e100880907cb 100644
>--- a/browser/base/content/tabbrowser.js
>+++ b/browser/base/content/tabbrowser.js
>@@ -887,6 +887,18 @@ window._gBrowser = {
>           aBrowser.currentURI);
>         if (uri.scheme == "about")
>           newTitle = uri.spec + sep + newTitle;
>+        else if (uri.scheme == "moz-extension") {
>+          // in the case of a WebExtension, we display its official name instead
>+          // the URL
>+          let mozExtensionHostname = uri.host;
>+          for (let extension of WebExtensionPolicy.getActiveExtensions()) {
>+            if (extension.mozExtensionHostname == mozExtensionHostname &&
>+                extension.name)
>+              return extension.name + sep + newTitle;
>+          }

Shouldn't need to loop...

          let extension = WebExtensionPolicy.getByHostname(uri.host);
          return `Extension (${extension.name})${sep}${newTitle}`;

Assuming some l10n will happen, so will wait for that.
Attachment #8970941 - Flags: review?(mixedpuppy)
Product: Toolkit → WebExtensions
Flags: needinfo?(emanuela)

Can we expect any progress on this issue in the near future?
I'm very keen on this 😉

Would it be OK to fix this by just using the same text the page indicator has in regular tabs, i.e. the Extension (<name>) bit, and reuse that?
I'd be interested in bringing this bug over the finish line.

Flags: needinfo?(mixedpuppy)

From my point of view, that'd be fine, Nils!

Although I'd prefer Extension: <name> as Andy suggested. But that's just details. Anything that makes the ugly moz-extension-prefix vanish would be a great improvement! 😊

With the current l18n string, which just is "Extension:" both alternatives are possible.
I guess it's up to mixedpuppy to weight in here which one to use.

Anything that makes the ugly moz-extension-prefix vanish would be a great improvement! 😊

ikr

(In reply to Nils Maier [:nmaier] from comment #22)

Would it be OK to fix this by just using the same text the page indicator has in regular tabs, i.e. the Extension (<name>) bit, and reuse that?
I'd be interested in bringing this bug over the finish line.

I think that's fine.

Flags: needinfo?(mixedpuppy)
Flags: qe-verify-
Keywords: checkin-needed

Pushed by ncsoregi@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b3b556ca3d5
Show extension name instead of URI in popup windows r=mixedpuppy

Keywords: checkin-needed

Backed out for failures on browser_ext_windows_popup_title.js

backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/d743f185d4d85634f85538abb4e85b467d1b4ec9

push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&revision=9b3b556ca3d5926ac8893b0fdedea8e7ca7bf9cb&selectedJob=265819206

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=265819206&repo=mozilla-inbound&lineNumber=10060

[task 2019-09-09T23:37:26.640Z] 23:37:26 INFO - TEST-INFO | started process screentopng
[task 2019-09-09T23:37:27.574Z] 23:37:27 INFO - TEST-INFO | screentopng: exit 0
[task 2019-09-09T23:37:27.575Z] 23:37:27 INFO - Buffered messages logged at 23:37:25
[task 2019-09-09T23:37:27.575Z] 23:37:27 INFO - Entering test bound test_popup_title
[task 2019-09-09T23:37:27.577Z] 23:37:27 INFO - Extension loaded
[task 2019-09-09T23:37:27.577Z] 23:37:27 INFO - Console message: Warning: attempting to write 7256 bytes to preference extensions.webextensions.uuids. This is bad for general performance and memory usage. Such an amount of data should rather be written to an external file. This preference will not be sent to any content processes.
[task 2019-09-09T23:37:27.577Z] 23:37:27 INFO - Buffered messages logged at 23:37:26
[task 2019-09-09T23:37:27.578Z] 23:37:27 INFO - TEST-PASS | toolkit/components/extensions/test/browser/browser_ext_windows_popup_title.js | poup title must include extension name -
[task 2019-09-09T23:37:27.578Z] 23:37:27 INFO - Buffered messages finished
[task 2019-09-09T23:37:27.584Z] 23:37:27 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/browser/browser_ext_windows_popup_title.js | popup title must include extension document title -
[task 2019-09-09T23:37:27.584Z] 23:37:27 INFO - Stack trace:
[task 2019-09-09T23:37:27.584Z] 23:37:27 INFO - chrome://mochikit/content/browser-test.js:test_ok:1580
[task 2019-09-09T23:37:27.584Z] 23:37:27 INFO - chrome://mochitests/content/browser/toolkit/components/extensions/test/browser/browser_ext_windows_popup_title.js:test_popup_title/<:40
[task 2019-09-09T23:37:27.584Z] 23:37:27 INFO - chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js:testMessage:79
[task 2019-09-09T23:37:27.584Z] 23:37:27 INFO - resource://specialpowers/SpecialPowersChild.jsm:listener:2030
[task 2019-09-09T23:37:27.584Z] 23:37:27 INFO - resource://specialpowers/SpecialPowersChild.jsm:loadExtension/<:1972
[task 2019-09-09T23:37:27.584Z] 23:37:27 INFO - resource://specialpowers/SpecialPowersChild.jsm:receiveMessage:238
[task 2019-09-09T23:37:27.584Z] 23:37:27 INFO - TEST-PASS | toolkit/components/extensions/test/browser/browser_ext_windows_popup_title.js | popup title must not include extension URL -
[task 2019-09-09T23:37:27.584Z] 23:37:27 INFO - Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "moz-extension://8118729b-e1ea-4923-bfc7-6d91ce4f9a37/index.html" line: 0}]
[task 2019-09-09T23:37:27.584Z] 23:37:27 INFO - GECKO(4556) | JavaScript error: resource:///modules/UrlbarInput.jsm, line 231: TypeError: this._inputFieldEvents is undefined
[task 2019-09-09T23:37:27.584Z] 23:37:27 INFO - Console message: [JavaScript Error: "TypeError: this._inputFieldEvents is undefined" {file: "resource:///modules/UrlbarInput.jsm" line: 231}]
[task 2019-09-09T23:37:27.584Z] 23:37:27 INFO - TEST-PASS | toolkit/components/extensions/test/browser/browser_ext_windows_popup_title.js | popup-window-title -

Flags: needinfo?(maierman)

This is why I hate writing browser tests :p

Flags: needinfo?(maierman)

mixedpuppy, can you have another looksy please?

Flags: needinfo?(mixedpuppy)

commented in patch

Flags: needinfo?(mixedpuppy)
Assignee: nobody → maierman
Flags: in-testsuite+
Keywords: checkin-needed

Pushed by dluca@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6d401ddd9ce0
Show extension name instead of URI in popup windows r=mixedpuppy

Keywords: checkin-needed

https://hg.mozilla.org/mozilla-central/rev/6d401ddd9ce0

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox71: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
See Also: → 1693664
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: