Closed Bug 1215487 Opened 4 years ago Closed 4 years ago

The mozilla logo isn't displayed on the standalone UI in Google Chrome

Categories

(Hello (Loop) :: Client, defect, P1)

defect

Tracking

(firefox45 verified)

VERIFIED FIXED
mozilla45
Iteration:
45.1 - Nov 16
Tracking Status
firefox45 --- verified

People

(Reporter: standard8, Assigned: dcritchley)

Details

(Keywords: regression)

Attachments

(3 files, 3 obsolete files)

Checking out latest fx-team and loading the standalone UI in Google Chrome reveals that the Mozilla logo isn't displayed as it is in Firefox.

The issue is due to the use of svg fragment identifiers and that fact that Chrome doesn't like these as background-images (known Chrome issue).

We need to change the logo to be displayed via an image tag or something.
P1 as this is needed before we can do the next major release of standalone.
Rank: 9
Hi Mark, lowering the priority on that one since my understanding is that this is about not displaying the Mozilla logo (i.e lower priority than the rest of the user journey)
Rank: 9 → 20
Attached image screenshot of problem
top chrome, bottom firefox
Assignee: nobody → david.critchley
Fix Mozilla logo in Chrome for Loop Standalone
Attachment #8681491 - Flags: review?(edilee)
Comment on attachment 8681491 [details] [diff] [review]
Attachment to Bug 1215487 - The mozilla logo isn't displayed on the standalone UI in Google Chrome

>+++ b/browser/components/loop/standalone/content/css/webapp.css
> .mozilla-logo {
>-  background: url("../img/mozilla-logo.svg#logo") no-repeat;
>-  background-size: contain;
>   width: 100px;
>-  height: 30px;
>+  height: 26px;
> }
What does this change do? I don't see an accompanying <img>
Flags: needinfo?(david.critchley)
Fix Mozilla logo in Chrome for Loop Standalone
Attachment #8681491 - Attachment is obsolete: true
Attachment #8681491 - Flags: review?(edilee)
Attachment #8682034 - Flags: review?(edilee)
(In reply to Ed Lee :Mardak from comment #6)
> Comment on attachment 8681491 [details] [diff] [review]
> Attachment to Bug 1215487 - The mozilla logo isn't displayed on the
> standalone UI in Google Chrome
> 
> >+++ b/browser/components/loop/standalone/content/css/webapp.css
> > .mozilla-logo {
> >-  background: url("../img/mozilla-logo.svg#logo") no-repeat;
> >-  background-size: contain;
> >   width: 100px;
> >-  height: 30px;
> >+  height: 26px;
> > }
> What does this change do? I don't see an accompanying <img>

Yes, found other instance of mozlogo which needed to change as well. Thanks for catching this.
Flags: needinfo?(david.critchley)
Comment on attachment 8682034 [details] [diff] [review]
Attachment to Bug 1215487 - The mozilla logo isn't displayed on the standalone UI in Google Chrome

Can you attach a before/after screenshot for each of those? I'm wondering how changing it from a p/div/block to a img/inline affects the layout.
Flags: needinfo?(david.critchley)
Flags: needinfo?(david.critchley)
Comment on attachment 8682034 [details] [diff] [review]
Attachment to Bug 1215487 - The mozilla logo isn't displayed on the standalone UI in Google Chrome

>+++ b/browser/components/loop/standalone/content/js/standaloneRoomViews.jsx
>@@ -117,17 +117,17 @@ loop.standaloneRoomViews = (function(mozL10n) {
>             <ToSView dispatcher={this.props.dispatcher} />
>-            <p className="mozilla-logo" />
>+            <img className="mozilla-logo" src="shared/img/mozilla-logo.svg#logo" />
I don't think Chrome users would normally see StandaloneHandleUserAgentView as I believe that's supposed to be for Firefoxes that can open a chat window for their own chat instead of taking a spot as a guest in the standalone.

But it's good practice to avoid svg background images.
Attachment #8682034 - Flags: review?(edilee) → review+
I'll check this in.
Comment on attachment 8682034 [details] [diff] [review]
Attachment to Bug 1215487 - The mozilla logo isn't displayed on the standalone UI in Google Chrome

>rename from browser/components/loop/standalone/content/img/mozilla-logo.svg
>rename to browser/components/loop/content/shared/img/mozilla-logo.svg
Sorry missed this earlier. Why the move?
Flags: needinfo?(david.critchley)
Attachment #8682034 - Flags: review+ → review?(edilee)
I couldn't reference the svg from the jsx pages in the standalone folder, so looking at other examples they showed the logos/images being referenced from the content shared folder, so moved the svg there.
Flags: needinfo?(david.critchley)
Comment on attachment 8682034 [details] [diff] [review]
Attachment to Bug 1215487 - The mozilla logo isn't displayed on the standalone UI in Google Chrome

>+          <img className="standalone-moz-logo" src="shared/img/mozilla-logo.svg#logo-white" />
The simplest approach of doing src="img/mozilla-logo.svg#logo-white" works. There's no need to move the file.
Attachment #8682034 - Flags: review?(edilee) → review-
(In reply to Ed Lee :Mardak from comment #15)
> >+          <img className="standalone-moz-logo" src="shared/img/mozilla-logo.svg#logo-white" />
> The simplest approach of doing src="img/mozilla-logo.svg#logo-white" works.
> There's no need to move the file.
The above won't quite work for ui showcase.

Standard8, do we have a good way to have non-css images work in both /ui and standalone /content?

Would it be reasonable to use absolute urls, e.g., <img src="/content/img/mozilla-logo.svg" /> ?
Flags: needinfo?(standard8)
(In reply to Ed Lee :Mardak from comment #16)
> (In reply to Ed Lee :Mardak from comment #15)
> > >+          <img className="standalone-moz-logo" src="shared/img/mozilla-logo.svg#logo-white" />
> > The simplest approach of doing src="img/mozilla-logo.svg#logo-white" works.
> > There's no need to move the file.
> The above won't quite work for ui showcase.
> 
> Standard8, do we have a good way to have non-css images work in both /ui and
> standalone /content?
> 
> Would it be reasonable to use absolute urls, e.g., <img
> src="/content/img/mozilla-logo.svg" /> ?

We can't include content/ in there as the production code doesn't know about it. When we move to an add-on, this is definitely one reason why we should change.

I think the best way to manage this for now would be to use img/mozilla-logo.svg#etc and then add an extra line into standalone/server.js:

app.use("/ui/img/", express.static(path.join(__dirname, "..", "standalone", "content", "img")));

or something like that.

It'd at least get the showcase working for now, and we can sort out the directories better later on (we already have a bug on this).
Flags: needinfo?(standard8)
Fix Mozilla logo in Chrome for Loop Standalone
Attachment #8682034 - Attachment is obsolete: true
Attachment #8682644 - Flags: review?(edilee)
Comment on attachment 8682644 [details] [diff] [review]
Attachment to Bug 1215487 - The mozilla logo isn't displayed on the standalone UI in Google Chrome

Comment 11 said that while Chrome doesn't see StandaloneHandleUserAgentView, we should fix it for consistency. Hopefully that'll avoid future bugs with Chrome + svg background because all existing examples use <img>.

>+++ b/browser/components/loop/standalone/server.js
>@@ -66,16 +66,19 @@ app.get("/content/c/config.js", getConfigFile);
> // /ui - for the ui showcase
> // /content - for the standalone files.
> 
> app.use("/ui", express.static(path.join(__dirname, "..", "ui")));
> app.use("/ui/loop/", express.static(path.join(__dirname, "..", "content")));
> app.use("/ui/shared/", express.static(path.join(__dirname, "..", "content",
>                                                 "shared")));
> 
>+// Exists for use by UI Showcase
>+app.use("/ui/img/", express.static(path.join(__dirname, "..", "standalone", "content", "img")));
This comment is unnecessary and move the app.use to just above the /ui/loop/ line from a few lines earlier.
Attachment #8682644 - Flags: review?(edilee) → review-
Also as a heads up, you can test it in chrome using the ui showcase, e.g.,

Standalone Room Handle Join in Firefox
Standalone room conversation (ready)
Fix Mozilla logo in Chrome for Loop Standalone
Attachment #8682644 - Attachment is obsolete: true
Attachment #8682765 - Flags: review?(edilee)
Comment on attachment 8682765 [details] [diff] [review]
Attachment to Bug 1215487 - The mozilla logo isn't displayed on the standalone UI in Google Chrome

I'll land this.
Attachment #8682765 - Flags: review?(edilee) → review+
https://hg.mozilla.org/mozilla-central/rev/117a37db2aa5
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Iteration: --- → 45.1 - Nov 16
Flags: qe-verify+
QA Contact: bogdan.maris
Verified against stage server with latest 45.0a1 (from 2015-11-08), across platforms [1] - the logo is correctly displayed in the standalone page via Chrome.

[1] Windows 7 64-bit, Mac OS X 10.11.1 and Ubuntu 14.04 64-bit
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.