1228478 - Replace sync-illustration.png with sync-illustration.svg

Status: RESOLVED FIXED

(Firefox :: Sync, defect, P3)

Description

[Mark Hammond [:markh] [:mhammond]]

Bug 1201331 landed an SVG image of the "sync illustration" used in the Sync preferences pane - although the colors are slightly different.

We should replace the use of the png files with the svg and use CSS to adjust the colors as required.

Comment 1

[chaithanya]

Hello, I would like to work on this bug. Could you please guide me through?

Comment 2

[Mark Hammond [:markh] [:mhammond]]

There are 3 copies of a "sync-illustration" - 2 are .png files:
 browser/themes/shared/fxa/sync-illustration.png
 browser/themes/shared/fxa/[e-mail]

The second of which is used in high-DPI displays. You can find their references via https://dxr.mozilla.org/mozilla-central/search?q=regexp%3Async-illustration.*.png

There is now a .svg version of the same image - browser/themes/shared/fxa/sync-illustration.svg - but it has a slighly different color than the .png images - the color is specified in the svg file. The task is:

* replace the references to the 2 .png files with a single reference to the .svg (the reference to the @2x version can be removed completely, as the svg will scale correctly on high-DPI displays)
* use CSS so that the new references have the same color as the .png.
* Ensure the references to the SVG specify an appropriate size so that it looks identical when using the svg as when it doesn't.

Comment 3

[Vishnu (:malayaleecoder)]

@Mark , I would like to be assigned this bug. I have finished replacing the references and I am only left to change the color and size of .svg file similar to .png image.

Comment 4

[Chris Karlof [:ckarlof]]

Thanks malayaleecoder! I've assigned you.

Comment 5

[Vishnu (:malayaleecoder)]

I am done with changing the color of .svg file similar to .png file, but I am unable to get the idea of specifying the size of .svg file while referencing it.

Comment 6

[Mark Hammond [:markh] [:mhammond]]

(In reply to malayaleecoder from comment #5)
> I am done with changing the color of .svg file similar to .png file,

Please see comment 0 - the colors in the SVG file should not be changed, but instead CSS should be used.

> but I am unable to get the idea of specifying the size of .svg file while
> referencing it.

You should be able to specify an explicit width and height in the XUL <image> tag which shows this image - see https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/image

Comment 7

[Vishnu (:malayaleecoder)]

Attachment: Bug1228478_v1.diff

Hello Mark,

I have done everything except the change of color using CSS. I have searched various resources and am unable to implement it. Can you please help me out?

Cheers!

Comment 8

[Mark Hammond [:markh] [:mhammond]]

https://developer.mozilla.org/en-US/docs/Web/Guide/CSS/Getting_started/SVG_and_CSS should give you some help. Note that I think me might need to move away from using list-style-image here (ie, we might need to use an <image> tag to have the CSS applied) - the downside of that is that skins or themes will be unable to change that illustration, but I think that's OK here.

Comment 9

[Vishnu (:malayaleecoder)]

Mark, I tried the same way as in the article you had sent and there are no improvements :( I think the CSS is not able to link to the svg file. Also from what I have noticed, sync-illustration.svg is just used in one other place,

https://dxr.mozilla.org/mozilla-central/source/browser/themes/shared/customizableui/panelUIOverlay.inc.css#753

So, is it fine I change the fill attribute in the svg image itself? If not, please suggest me some other way around this.

Comment 10

[Mark Hammond [:markh] [:mhammond]]

(In reply to malayaleecoder from comment #9)
> So, is it fine I change the fill attribute in the svg image itself? If not,
> please suggest me some other way around this.

We don't want to change the color in that other place. The article I linked at (http://webdesign.tutsplus.com/articles/manipulating-svg-icons-with-simple-css--webdesign-15694) demonstrates how to change the fill color in an SVG from CSS (although it will possibly require some adjustment to the svg) - can you please post your work-in-progress so we can try and see what is going on?

Comment 11

[Dorel Barbu]

Hello, I've just started my journey here at Mozilla. 
I have been looking for a bug that I thought I could solve. Since the last comment here was in May, I thought I might give it a chance. Here's what I've done so far.
*I have obtained the color of the png file using http://labs.tineye.com. And I got: #bfcbd3
*I have managed to find out how to modify the color of sync-illustration.svg using an external css file. And it works, if I open just the svg file using the browser it displays the right color. 
*I have replaced all occurences of sync-illustration.png and [e-mail] with sync-illustration.png. The I did a build and it loads the svg file, but with the color unmodified.

Now, I'm trying to add the css to the source code, to change the color of the svg using css, but I'm struggling with this. I've done some research and the best option seems to be to use svg as an <object>. But now I'm stuck. Here's why:

*I thought that making changes to /home/dorel/src/mozilla-central/browser/components/preferences/in-content/sync.xul would do the trick. I have found the line of code which displays the image.
*But I can't replace it with an <object> because, obviously sync.xul is not a html file and the changes wouldn't take effect.

Is my approach good? And if it is, could somebody guide me from here on, please? If not, a hint maybe..? 

I saw that this bug was assigned but I also saw that there was no activity. Sorry if I shouldn't have comment.

Comment 12

[Vishnu (:malayaleecoder)]

Sorry for not looking into this for a long time (I had forgotten about it). Removing the assignee.

Comment 13

[Dorel Barbu]

(In reply to Vishnu (:malayaleecoder) from comment #12)
> Sorry for not looking into this for a long time (I had forgotten about it).
> Removing the assignee.

Then, could you please assign me? I'm already working on it and exploring additional solutions.

Comment 14

[Vishnu (:malayaleecoder)]

(In reply to Dorel Barbu from comment #13)
> (In reply to Vishnu (:malayaleecoder) from comment #12)
> > Sorry for not looking into this for a long time (I had forgotten about it).
> > Removing the assignee.
> 
> Then, could you please assign me? I'm already working on it and exploring
> additional solutions.

Sure, here you go. Welcome to Mozilla :)

Comment 15

[Dorel Barbu]

(In reply to Vishnu (:malayaleecoder) from comment #14)
> (In reply to Dorel Barbu from comment #13)
> > (In reply to Vishnu (:malayaleecoder) from comment #12)
> > > Sorry for not looking into this for a long time (I had forgotten about it).
> > > Removing the assignee.
> > 
> > Then, could you please assign me? I'm already working on it and exploring
> > additional solutions.
> 
> Sure, here you go. Welcome to Mozilla :)

Thank you very much! Happy New Year!  Beginning with the January 2nd I'll resume my work. Let's enjoy this last free day :)

Comment 16

[Mark Hammond [:markh] [:mhammond]]

(In reply to Dorel Barbu from comment #11)
> *I thought that making changes to
> /home/dorel/src/mozilla-central/browser/components/preferences/in-content/
> sync.xul would do the trick. I have found the line of code which displays
> the image.
> *But I can't replace it with an <object> because, obviously sync.xul is not
> a html file and the changes wouldn't take effect.

You should be able to use, eg, <html:img> in a xul file similar to https://dxr.mozilla.org/mozilla-central/source/devtools/client/webide/content/webide.xul#128 - does that help?

Comment 17

[Dorel Barbu]

(In reply to Mark Hammond [:markh] from comment #16)
> (In reply to Dorel Barbu from comment #11)
> > *I thought that making changes to
> > /home/dorel/src/mozilla-central/browser/components/preferences/in-content/
> > sync.xul would do the trick. I have found the line of code which displays
> > the image.
> > *But I can't replace it with an <object> because, obviously sync.xul is not
> > a html file and the changes wouldn't take effect.
> 
> You should be able to use, eg, <html:img> in a xul file similar to
> https://dxr.mozilla.org/mozilla-central/source/devtools/client/webide/
> content/webide.xul#128 - does that help?

Thank you, this is helpful. But I've been unsuccessful. Apparently I can't use <html:img because it doesn't load the image at all. If I put a <html: table created by me, it loads the html element. I'm thinking that I didn't set correctly the src attribute of the image, but I'm still trying and I won't give up. Any other suggestion, Mark?

Comment 18

[Mark Hammond [:markh] [:mhammond]]

Edouard, do you think you could help Dorel with this (and note that your bug 1196153 will mean another place where we can hopefully replace the png with svg)

Comment 19

[Dorel Barbu]

(In reply to Mark Hammond [:markh] from comment #18)
> Edouard, do you think you could help Dorel with this (and note that your bug
> 1196153 will mean another place where we can hopefully replace the png with
> svg)

Any help would be greatly appreciated. Thank your for your patience, Mark! Today I've started my work from scratch to try and figure why my image isn't displayed with the <html:img> tag.

Comment 20

[Edouard Oger [:eoger]]

I was able to display the SVG using an <html:img> tag.
However, it is not possible to change the fill color of an SVG using CSS when it is not inlined in the document, but that would lead to code duplication everywhere.
After investigation, what I would probably suggest is to have two different fill colors defined in the SVG that can be activated depending on the hash in the URL. See here: https://gist.github.com/LeaVerou/5198257

Comment 21

[Dorel Barbu]

(In reply to Edouard Oger [:eoger] from comment #20)
> I was able to display the SVG using an <html:img> tag.
> However, it is not possible to change the fill color of an SVG using CSS
> when it is not inlined in the document, but that would lead to code
> duplication everywhere.
> After investigation, what I would probably suggest is to have two different
> fill colors defined in the SVG that can be activated depending on the hash
> in the URL. See here: https://gist.github.com/LeaVerou/5198257

I am looking now. In the meantime, check this, please: https://github.com/DorelBarbu/Test-svg-image/tree/master. I have managed to write a simple html file in which I put an object tag. Then, the svg image was styled using an external stylesheet. 

Which approach would be better? The one with the hash, or this one?

Comment 22

[Edouard Oger [:eoger]]

You approach with the <object> element seems reasonable, unless Mark objects I think this could be fine.

Comment 23

[Dorel Barbu]

(In reply to Edouard Oger [:eoger] from comment #22)
> You approach with the <object> element seems reasonable, unless Mark objects
> I think this could be fine.

Mark, what do you think? By the way, forgive me if I ask too many questions but this is my first bug and I want to do the things as good as possible. Thank you :)

Comment 24

[Mark Hammond [:markh] [:mhammond]]

I can't see any good reason not to use <html:object> if it works with XUL.

Comment 25

[Dorel Barbu]

(In reply to Mark Hammond [:markh] from comment #24)
> I can't see any good reason not to use <html:object> if it works with XUL.

Ok then, I'm proceeding with this approach now.

Comment 26

[Dorel Barbu]

I am one step away from solving it. I've been  spending around five hours working on it before asking this question but I belive that I miss something because it doesn't make sense. 

I've been toying with the code a little, trying to get a feel of it. I've managed to add an object using the <html: object>. For example, if I insert the following code, then it works flawlessly.

<html:object xmlns:html="http://www.w3.org/1999/xhtml" type="image/svg+xml" data="https://upload.wikimedia.org/wikipedia/en/thumb/e/e3/Firefox-logo.svg/1072px-Firefox-logo.svg.png"></html:object>

The trouble comes when I try to change the data attribute. Simply putting browser/themes/shared/fxa/sync-illustration.svg doesn't work. I figured that the problem is that I set it incorrectly and I don't understand how to properly refer to a certain path. I looked more carefully at some other code of the Firefox browser and I saw that all the src attributes are set using chrome://... But I didn't quite understand how we're doing this. Reading https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Tutorial/The_Chrome_URL didn't clarify my problem, but maybe I haven't looked closely enough.

Could you please give me a hint about how do I set correctly the data attribute? How do I make the 'transformation' to a 'chrome://...' path?

Comment 27

[Dorel Barbu]

I am sorry, I've figured it out in the meantime. I undertood what does chrome:// means and I've just managed to display my image the way I wanted. No I'm adding the css part. I'll soon post an update on my progress. Sorry. I can't find the ability to delete/edit comments. I apologize.

Comment 28

[Edouard Oger [:eoger]]

No problem Dorel it's part of the learning process, I'm glad you found the solution.
Bug 1228478 just landed on mozilla-central, which means there is one more place where you will have to replace the png by the svg.

Comment 29

[Edouard Oger [:eoger]]

Woops I meant bug 1196153

Comment 30

[Dorel Barbu]

Attachment: Bug 1228478 - Modified the svg such that it can be styled without an external stylesheet. Removed all references of the pngs.

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

Comment 31

[Dorel Barbu]

I mangaed to push my changes!! I can't wait the review!! If everything is fine, I'll take on that bug, Edouard. But I've seen that you are already assigned to it. Can I also work on it? I'm a newbie here.

Comment 32

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8824776 [details]
Bug 1228478 - Modified the svg such that it can be styled without an external stylesheet. Removed all references of the pngs.

https://reviewboard.mozilla.org/r/103082/#review103702

::: browser/base/content/aboutaccounts/main.css:113
(Diff revision 1)
>    background: #08c;
>  }
>  
>  
>  .graphic-sync-intro {
> -  background-image: url(chrome://browser/skin/fxa/sync-illustration.png);
> +  height: 195px;

Does this change cause the layout to change? ie, is the size of this element difference after this patch from what it was before?

::: browser/base/content/aboutaccounts/main.css
(Diff revision 1)
>  /* Retina */
>  @media
>  only screen and (min-device-pixel-ratio: 2),
>  only screen and (        min-resolution: 192dpi),
>  only screen and (        min-resolution: 2dppx) {
> -  .graphic-sync-intro {

I think this entire block can be removed (ie, from line 154 to the end of the file).

::: browser/themes/shared/fxa/sync-illustration.svg:2
(Diff revision 1)
>  <?xml version="1.0" encoding="utf-8"?>
> +<?xml-stylesheet href="chrome://browser/skin/preferences/in-content/preferences.css" type="text/css"?>

I think we should see if we can work out how to not include the entire preferences.css here - I suspect that will cause some performance issues given the size of this css.

::: browser/themes/shared/fxa/sync-illustration.svg:7
(Diff revision 1)
> +<?xml-stylesheet href="chrome://browser/skin/preferences/in-content/preferences.css" type="text/css"?>
>  <!-- This Source Code Form is subject to the terms of the Mozilla Public
>     - License, v. 2.0. If a copy of the MPL was not distributed with this
>     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
>  <svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMidYMid" width="320" height="280" viewBox="0 0 320 280">
> -  <g fill="#cdcdcd">
> +  <g class="fxaSyncIllustrationSvg">

Note that this SVG should use a fill color of #cdcdcd in the places where it was already used, while it should have the new color in places where you are replacing the .png - I don't think that is currently happening.

::: browser/themes/shared/incontentprefs/preferences.inc.css:470
(Diff revision 1)
>  
>  .fxaSyncIllustration {
>    margin-top: 35px;
>  }
>  
> +.fxaSyncIllustrationSvg{

please put a space before the "{"

::: browser/themes/shared/incontentprefs/preferences.inc.css:519
(Diff revision 1)
>    padding-right: 11px;
>    margin: 0;
>    min-width: 0;
>  }
>  
> +.svgFxaSyncIllustration

please put the opening brace on the line above to match the other lines - but I'm not sure what this line is for - that class doesn't seem used.

::: browser/themes/shared/jar.inc.mn
(Diff revision 1)
>  * skin/classic/browser/preferences/in-content/containers.css   (../shared/incontentprefs/containers.css)
>  * skin/classic/browser/preferences/containers.css              (../shared/preferences/containers.css)
>    skin/classic/browser/fxa/default-avatar.svg                  (../shared/fxa/default-avatar.svg)
>    skin/classic/browser/fxa/logo.png                            (../shared/fxa/logo.png)
>    skin/classic/browser/fxa/[e-mail]                         (../shared/fxa/[e-mail])
> -  skin/classic/browser/fxa/sync-illustration.png               (../shared/fxa/sync-illustration.png)

Note that this patch should remove the .png files themselves too as they should no longer be referenced anywhere after this patch.

Comment 33

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8824776 [details]
Bug 1228478 - Modified the svg such that it can be styled without an external stylesheet. Removed all references of the pngs.

https://reviewboard.mozilla.org/r/103082/#review103704

Thanks for the patch! This patch looks like it is heading in the right direction but there are some issues I've listed below. When you put up the next version of the patch, could you also please upload some before-and-after screenshots of the various places this patch touches - it's important there are no visual differences.

Comment 34

[Dorel Barbu]

Attachment: Deleted pngs

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

Comment 35

[Dorel Barbu]

Attachment: Bug 1228478 - Removed unncesessary code from main.css

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

Comment 36

[Dorel Barbu]

Attachment: Fixed codding style in preferences.inc.css and removed unnecessary class. Removed the pngs in themes/shared/fxa

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

Comment 37

[Dorel Barbu]

Attachment: Modified code and styling of svg.

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

Comment 38

[Dorel Barbu]

(In reply to Mark Hammond [:markh] from comment #33)
> Comment on attachment 8824776 [details]
> Bug 1228478 - Modified svg illustration such that it can be styled. Added
> coresponding styles. Replaced all the png with the svg. The svg file is
> placed using html:object tab.
> 
> https://reviewboard.mozilla.org/r/103082/#review103704
> 
> Thanks for the patch! This patch looks like it is heading in the right
> direction but there are some issues I've listed below. When you put up the
> next version of the patch, could you also please upload some
> before-and-after screenshots of the various places this patch touches - it's
> important there are no visual differences.

Thank you, Mark! I've done an update. I apologize, apparently I've forgotten to commit some changes the first time I did the hg push review. I have a few questions:

1. This time I did many commits, one for each issue you commented. Is this approach ok? 
2. How do you suggest dealing with the preferences.inc.css? I wouldadd a new stylesheet which styles only the svg. Is this ok?

I'm waiting for your opinion.

Comment 39

[Dorel Barbu]

(In reply to Mark Hammond [:markh] from comment #33)
> Comment on attachment 8824776 [details]
> Bug 1228478 - Modified svg illustration such that it can be styled. Added
> coresponding styles. Replaced all the png with the svg. The svg file is
> placed using html:object tab.
> 
> https://reviewboard.mozilla.org/r/103082/#review103704
> 
> Thanks for the patch! This patch looks like it is heading in the right
> direction but there are some issues I've listed below. When you put up the
> next version of the patch, could you also please upload some
> before-and-after screenshots of the various places this patch touches - it's
> important there are no visual differences.

I'l come back with some screenshots :)

Comment 40

[Edouard Oger [:eoger]]

Comment on attachment 8824776 [details]
Bug 1228478 - Modified the svg such that it can be styled without an external stylesheet. Removed all references of the pngs.

https://reviewboard.mozilla.org/r/103080/#review103870

::: browser/base/content/aboutaccounts/main.css:112
(Diff revision 3)
>  .button-row button:focus {
>    background: #08c;
>  }
>  
>  
>  .graphic-sync-intro {

Do we still use this class now? If not the whole block can be removed.

::: browser/themes/shared/incontentprefs/preferences.inc.css:470
(Diff revision 3)
>  
>  .fxaSyncIllustration {
>    margin-top: 35px;
>  }
>  
> +.svg {

This class name is too general, try to be more specific. ie: something like fxaSyncIllustration-fill

Comment 41

[Edouard Oger [:eoger]]

> 1. This time I did many commits, one for each issue you commented. Is this approach ok? 

I would probably rebase the same commit, reviewboard handles multiple versions of the same commit contrary to Github.

Comment 42

[Dorel Barbu]

Thanks for reviewing my code, Edouard! I don't know what is a comment rebase, but I will find out. I'm sorry if my commits aren't the way they're supposed to be. Yes, there are some other things to modify, as you noticed. Unfortunately, I can't make the changes untill noon. I had to reinstall my laptop, long story. :( 

Here are the screenshots. I uploaded them via google drive:

Using PNG: https://drive.google.com/open?id=0B1XsODm48EQ4LU1hSmJxNDBMZVk
Using SVG: https://drive.google.com/open?id=0B1XsODm48EQ4WktxV2txZEt2QkU

What do you guys think? It looks good to me. Although there is a tiny difference I believe i's because the two images scale differently.

Comment 43

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8824776 [details]
Bug 1228478 - Modified the svg such that it can be styled without an external stylesheet. Removed all references of the pngs.

https://reviewboard.mozilla.org/r/103080/#review104424

Thanks for your continued work on this! As mentioned in the bug comments, please look into |hg rebase| for how you can squash these fixups into a single patch, then address both my and eoger's comments and push a new patch up

::: browser/base/content/aboutaccounts/main.css:112
(Diff revision 3)
>  .button-row button:focus {
>    background: #08c;
>  }
>  
>  
>  .graphic-sync-intro {

Alternatively, maybe we *should* be using it so that the size is specified? We probably need a screenshot of about:accounts to be sure.

::: browser/themes/shared/fxa/sync-illustration.svg:2
(Diff revision 3)
>  <?xml version="1.0" encoding="utf-8"?>
> +<?xml-stylesheet href="chrome://browser/skin/preferences/in-content/preferences.css" type="text/css"?>

As I mentioned before, it would be great if we can avoid dragging in this large style-sheet here.

::: browser/themes/shared/fxa/sync-illustration.svg:2
(Diff revision 3)
>  <?xml version="1.0" encoding="utf-8"?>
> +<?xml-stylesheet href="chrome://browser/skin/preferences/in-content/preferences.css" type="text/css"?>

As I mentioned before, we shouldn't be pulling this css in here because (a) it's too big and may affect performance and (b) this SVG is currently used in places other than "preferences".

::: browser/themes/shared/fxa/sync-illustration.svg:7
(Diff revision 3)
> +<?xml-stylesheet href="chrome://browser/skin/preferences/in-content/preferences.css" type="text/css"?>
>  <!-- This Source Code Form is subject to the terms of the Mozilla Public
>     - License, v. 2.0. If a copy of the MPL was not distributed with this
>     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
>  <svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMidYMid" width="320" height="280" viewBox="0 0 320 280">
> -  <g fill="#cdcdcd">
> +  <g class="svg">

Also as mentioned before, I expect there to be somewhere that #cdcdcd is specified in a stylesheet so the existing uses of the svg use the same color they do now - eg, I expect that when this illustration is shown in the "synced tabs" menu panel the color will be different.

::: browser/themes/shared/incontentprefs/preferences.inc.css
(Diff revision 3)
>    margin-inline-end: 14px !important;
>  }
>  
>  .fxaSyncIllustration {
>    width: 231px;
> -  list-style-image: url(chrome://browser/skin/fxa/sync-illustration.png)

I wonder if specifying the height here will fix the issue of the image being a slightly different size in your screenshots?

Comment 44

[Dorel Barbu]

(In reply to Mark Hammond [:markh] from comment #43)
> Comment on attachment 8824776 [details]
> Bug 1228478 - Modified svg illustration such that it can be styled. Added
> coresponding styles. Replaced all the png with the svg. The svg file is
> placed using html:object tab.
> 
> https://reviewboard.mozilla.org/r/103080/#review104424
> 
> Thanks for your continued work on this! As mentioned in the bug comments,
> please look into |hg rebase| for how you can squash these fixups into a
> single patch, then address both my and eoger's comments and push a new patch
> up
> 
> ::: browser/base/content/aboutaccounts/main.css:112
> (Diff revision 3)
> >  .button-row button:focus {
> >    background: #08c;
> >  }
> >  
> >  
> >  .graphic-sync-intro {
> 
> Alternatively, maybe we *should* be using it so that the size is specified?
> We probably need a screenshot of about:accounts to be sure.
> 
> ::: browser/themes/shared/fxa/sync-illustration.svg:2
> (Diff revision 3)
> >  <?xml version="1.0" encoding="utf-8"?>
> > +<?xml-stylesheet href="chrome://browser/skin/preferences/in-content/preferences.css" type="text/css"?>
> 
> As I mentioned before, it would be great if we can avoid dragging in this
> large style-sheet here.
> 
> ::: browser/themes/shared/fxa/sync-illustration.svg:2
> (Diff revision 3)
> >  <?xml version="1.0" encoding="utf-8"?>
> > +<?xml-stylesheet href="chrome://browser/skin/preferences/in-content/preferences.css" type="text/css"?>
> 
> As I mentioned before, we shouldn't be pulling this css in here because (a)
> it's too big and may affect performance and (b) this SVG is currently used
> in places other than "preferences".
> 
> ::: browser/themes/shared/fxa/sync-illustration.svg:7
> (Diff revision 3)
> > +<?xml-stylesheet href="chrome://browser/skin/preferences/in-content/preferences.css" type="text/css"?>
> >  <!-- This Source Code Form is subject to the terms of the Mozilla Public
> >     - License, v. 2.0. If a copy of the MPL was not distributed with this
> >     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> >  <svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMidYMid" width="320" height="280" viewBox="0 0 320 280">
> > -  <g fill="#cdcdcd">
> > +  <g class="svg">
> 
> Also as mentioned before, I expect there to be somewhere that #cdcdcd is
> specified in a stylesheet so the existing uses of the svg use the same color
> they do now - eg, I expect that when this illustration is shown in the
> "synced tabs" menu panel the color will be different.
> 
> ::: browser/themes/shared/incontentprefs/preferences.inc.css
> (Diff revision 3)
> >    margin-inline-end: 14px !important;
> >  }
> >  
> >  .fxaSyncIllustration {
> >    width: 231px;
> > -  list-style-image: url(chrome://browser/skin/fxa/sync-illustration.png)
> 
> I wonder if specifying the height here will fix the issue of the image being
> a slightly different size in your screenshots?

Thank you, Mark, I'll look into it very soon. But I still have a question. Excluding  the CSS reference from the SVG doesn't work.  The SVG file shouldn't contain any css in it? Not even a smaller one?

Comment 45

[Mark Hammond [:markh] [:mhammond]]

(In reply to Dorel Barbu from comment #44)
> Thank you, Mark, I'll look into it very soon. But I still have a question.
> Excluding  the CSS reference from the SVG doesn't work.

I'm not sure what you mean by "doesn't work" here.

>  The SVG file
> shouldn't contain any css in it? Not even a smaller one?

The fill color is the only issue here, and I don't think it should need any external css references. What we need is the stylesheet used in the content that hosts the CSS to have rules that override the fill color. Something like https://css-tricks.com/cascading-svg-fill-color/, although it's not immediately clear that will actually work with <object>, you will need to experiment to know. If you have problems, comment 20 might have a solution.

Comment 46

[Dorel Barbu]

(In reply to Mark Hammond [:markh] from comment #45)
> (In reply to Dorel Barbu from comment #44)
> > Thank you, Mark, I'll look into it very soon. But I still have a question.
> > Excluding  the CSS reference from the SVG doesn't work.
> 
> I'm not sure what you mean by "doesn't work" here.
> 
> >  The SVG file
> > shouldn't contain any css in it? Not even a smaller one?
> 
> The fill color is the only issue here, and I don't think it should need any
> external css references. What we need is the stylesheet used in the content
> that hosts the CSS to have rules that override the fill color. Something
> like https://css-tricks.com/cascading-svg-fill-color/, although it's not
> immediately clear that will actually work with <object>, you will need to
> experiment to know. If you have problems, comment 20 might have a solution.

Thank you for your explanations. Now I have a better idea.

I'll work on it, and I'll see how I can get rid of that css. I'll soon post an update on the patch. I've looked into hg rebase and I saw what it can do. Hope that the next patch will be an improvement. I'll post it around Monday, 23 January, because I have some exams that I need to tale care off.

Although at first I didn't know where to start, now I feel part of the community. Thank you!!

Comment 47

[Dorel Barbu]

(In reply to Mark Hammond [:markh] from comment #45)
> (In reply to Dorel Barbu from comment #44)
> > Thank you, Mark, I'll look into it very soon. But I still have a question.
> > Excluding  the CSS reference from the SVG doesn't work.
> 
> I'm not sure what you mean by "doesn't work" here.
> 
> >  The SVG file
> > shouldn't contain any css in it? Not even a smaller one?
> 
> The fill color is the only issue here, and I don't think it should need any
> external css references. What we need is the stylesheet used in the content
> that hosts the CSS to have rules that override the fill color. Something
> like https://css-tricks.com/cascading-svg-fill-color/, although it's not
> immediately clear that will actually work with <object>, you will need to
> experiment to know. If you have problems, comment 20 might have a solution.

Hello again! I've done some testing on a dummy HTML page and I have found that using something like this:

<svg> 
<use xlink:href="..."></use>
</svg> 

can yield the desired result.

There's no need to reference the CSS stylesheet inside the SVG image. We only need to put the appropriate stylesheet in the preferences.inc.css.

Mark, can you tell me if this approach is ok? I've tested it and it works fine. If it is, I'll proceed to work on a new patch.

Comment 48

[Mark Hammond [:markh] [:mhammond]]

(In reply to Dorel Barbu from comment #47)
> <svg> 
> <use xlink:href="..."></use>
> </svg> 
> 
> can yield the desired result.
...
> Mark, can you tell me if this approach is ok? I've tested it and it works
> fine. If it is, I'll proceed to work on a new patch.

It depends on the details, but I see no reason this can't work :)

Comment 49

[Dorel Barbu]

(In reply to Mark Hammond [:markh] from comment #48)
> (In reply to Dorel Barbu from comment #47)
> > <svg> 
> > <use xlink:href="..."></use>
> > </svg> 
> > 
> > can yield the desired result.
> ...
> > Mark, can you tell me if this approach is ok? I've tested it and it works
> > fine. If it is, I'll proceed to work on a new patch.
> 
> It depends on the details, but I see no reason this can't work :)

I've tried it. It generates some ugly code. We can't use that. 

Edward's solution, in comment 20, is very beautiful and elegant. I will follow that lead. But first I need to figure out how to correctly edit our svg illustration. I will add a <style> ... </style> inside the svg and build from there. I'll keep you posted.

Comment 50

[Dorel Barbu]

Comment on attachment 8824776 [details]
Bug 1228478 - Modified the svg such that it can be styled without an external stylesheet. Removed all references of the pngs.

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

Comment 51

[Dorel Barbu]

Comment on attachment 8824908 [details]
Deleted pngs

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

Comment 52

[Dorel Barbu]

Hello again!

I've commited my new changes. I've tried a new approach since I had to change the way the svg is styled. I kinda messed up the commits a little bit in the previous version. Sorry about that. I have used Edouard's 

I'm waiting for your review. Thank you both for your patience!

Comment 53

[Edouard Oger [:eoger]]

Comment on attachment 8824776 [details]
Bug 1228478 - Modified the svg such that it can be styled without an external stylesheet. Removed all references of the pngs.

https://reviewboard.mozilla.org/r/103080/#review107976

This is definitelly going in the right direction :)
I made a lot of comments, which are mostly about the form, but the approach is the right one.
You can also squash your 2 commits together, and rename the resulting one in the form of "Bug XXX - Commit msg". The commit msg should say what your patch does, instead of explaining how it does it.
Great work and I'll be waiting for your next patch revision!

::: browser/base/content/aboutaccounts/aboutaccounts.xhtml:45
(Diff revision 4)
>          <header>
>            <div id="email"></div>
>          </header>
>  
>          <section>
> -            <div class="graphic graphic-sync-intro"> </div>
> +            <html:object class="fxaSyncIllustration" data="chrome://browser/skin/fxa/sync-illustration.svg#newColor"/>

I'm getting an error trying to load the page.
You can drop the html: prefixes since it's already an html document.

::: browser/components/customizableui/content/panelUI.inc.xul:134
(Diff revision 4)
>              </vbox>
>              <!-- Sync is ready to Sync but the "tabs" engine isn't enabled-->
>              <hbox id="PanelUI-remotetabs-tabsdisabledpane" pack="center" flex="1">
>                <vbox class="PanelUI-remotetabs-instruction-box">
>                  <hbox pack="center">
> -                  <image class="fxaSyncIllustration" alt=""/>
> +                    <html:object class="fxaSyncIllustration" data="chrome://browser/skin/fxa/sync-illustration.svg#newColor"/>

In the "synced tabs" panel, we use the same illustration but with a different color (grey-greyish instead of blue-grayish), this is why we are setting up the #hash hack so you can switch colors easily here. (or maybe you can define a default fill color which is applied when we don't set-up the hash)

::: browser/themes/shared/fxa/sync-illustration.svg:4
(Diff revision 4)
> -<?xml version="1.0" encoding="utf-8"?>
> -<!-- This Source Code Form is subject to the terms of the Mozilla Public
> -   - License, v. 2.0. If a copy of the MPL was not distributed with this
> -   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +<?xml version="1.0" encoding="UTF-8" standalone="no"?>
> +<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
> +<svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMidYMid" width="320" height="280" viewBox="0 0 320 280" xmlns:xlink="http://www.w3.org/1999/xlink" >
> + 	<style>

Use (2) spaces for indentation

::: browser/themes/shared/fxa/sync-illustration.svg:5
(Diff revision 4)
> -<?xml version="1.0" encoding="utf-8"?>
> -<!-- This Source Code Form is subject to the terms of the Mozilla Public
> -   - License, v. 2.0. If a copy of the MPL was not distributed with this
> -   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> -<svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMidYMid" width="320" height="280" viewBox="0 0 320 280">
> +<?xml version="1.0" encoding="UTF-8" standalone="no"?>
> +<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
> +<svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMidYMid" width="320" height="280" viewBox="0 0 320 280" xmlns:xlink="http://www.w3.org/1999/xlink" >
> + 	<style>
> +	#newColor:target ~ use,

Reindent the whole CSS block to the right between the <style> tags

::: browser/themes/shared/fxa/sync-illustration.svg:10
(Diff revision 4)
> -<svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMidYMid" width="320" height="280" viewBox="0 0 320 280">
> -  <g fill="#cdcdcd">
> +	#newColor:target ~ use,
> +	#newColor:target ~ g {
> +		fill: #bfcbd3;
> +	}
> +
> +	svg{

Space between svg and {

::: browser/themes/shared/fxa/sync-illustration.svg:13
(Diff revision 4)
> +	}
> +
> +	svg{
> +		fill:#cdcdcd;
> +	}
> +

No newline here

::: browser/themes/shared/fxa/sync-illustration.svg:16
(Diff revision 4)
> +		fill:#cdcdcd;
> +	}
> +
> +</style>
> +<defs>
> +<g id="logo">

Reindent this since it's now inside <defs>

::: browser/themes/shared/fxa/sync-illustration.svg:26
(Diff revision 4)
>      <path d="M297.688,276.000 L102.312,276.000 C97.721,276.000 94.000,272.279 94.000,267.688 L94.000,260.000 L306.000,260.000 L306.000,267.688 C306.000,272.279 302.279,276.000 297.688,276.000 ZM117.906,150.312 C117.906,145.721 121.628,142.000 126.218,142.000 L273.688,142.000 C278.279,142.000 282.000,145.721 282.000,150.312 L282.000,256.000 L117.906,256.000 L117.906,150.312 ZM132.000,242.000 L270.000,242.000 L270.000,156.000 L132.000,156.000 L132.000,242.000 Z"/>
>      <path d="M307.074,115.969 L206.926,115.969 C203.101,115.969 200.000,112.868 200.000,109.042 L200.000,38.926 C200.000,35.101 203.101,32.000 206.926,32.000 L307.074,32.000 C310.899,32.000 314.000,35.101 314.000,38.926 L314.000,109.042 C314.000,112.868 310.899,115.969 307.074,115.969 ZM210.000,65.875 C210.000,64.770 209.105,63.875 208.000,63.875 C206.895,63.875 206.000,64.770 206.000,65.875 L206.000,82.000 C206.000,83.105 206.895,84.000 208.000,84.000 C209.105,84.000 210.000,83.105 210.000,82.000 L210.000,65.875 ZM302.000,42.000 L216.000,42.000 L216.000,106.000 L302.000,106.000 L302.000,42.000 Z"/>
>      <path d="M65.844,240.000 L26.156,240.000 C23.861,240.000 22.000,238.139 22.000,235.844 L22.000,162.156 C22.000,159.861 23.861,158.000 26.156,158.000 L65.844,158.000 C68.139,158.000 70.000,159.861 70.000,162.156 L70.000,235.844 C70.000,238.139 68.139,240.000 65.844,240.000 ZM46.000,236.000 C48.287,236.000 50.141,234.195 50.141,231.969 C50.141,229.742 48.287,227.938 46.000,227.938 C43.713,227.938 41.859,229.742 41.859,231.969 C41.859,234.195 43.713,236.000 46.000,236.000 ZM66.000,168.000 L26.000,168.000 L26.000,224.000 L66.000,224.000 L66.000,168.000 Z"/>
>      <path d="M171.906,86.156 C171.906,102.329 159.026,115.469 143.017,115.797 L143.039,115.955 L28.850,115.955 L28.869,115.797 C12.872,115.475 -0.000,102.333 -0.000,86.156 C-0.000,71.661 10.336,59.603 23.994,57.019 C23.620,55.457 23.401,53.834 23.401,52.156 C23.401,40.714 32.606,31.438 43.962,31.438 C47.561,31.438 50.941,32.375 53.884,34.012 C53.883,33.930 53.878,33.848 53.878,33.766 C53.878,17.137 67.301,3.656 83.858,3.656 C97.763,3.656 109.453,13.164 112.843,26.059 C116.677,23.334 121.343,21.719 126.393,21.719 C139.394,21.719 149.933,32.331 149.933,45.422 C149.933,49.572 148.868,53.468 147.007,56.861 C161.114,59.082 171.906,71.351 171.906,86.156 Z"/>
> -  </g>
> +</g> 
> +

New newline here

::: browser/themes/shared/fxa/sync-illustration.svg:29
(Diff revision 4)
>      <path d="M171.906,86.156 C171.906,102.329 159.026,115.469 143.017,115.797 L143.039,115.955 L28.850,115.955 L28.869,115.797 C12.872,115.475 -0.000,102.333 -0.000,86.156 C-0.000,71.661 10.336,59.603 23.994,57.019 C23.620,55.457 23.401,53.834 23.401,52.156 C23.401,40.714 32.606,31.438 43.962,31.438 C47.561,31.438 50.941,32.375 53.884,34.012 C53.883,33.930 53.878,33.848 53.878,33.766 C53.878,17.137 67.301,3.656 83.858,3.656 C97.763,3.656 109.453,13.164 112.843,26.059 C116.677,23.334 121.343,21.719 126.393,21.719 C139.394,21.719 149.933,32.331 149.933,45.422 C149.933,49.572 148.868,53.468 147.007,56.861 C161.114,59.082 171.906,71.351 171.906,86.156 Z"/>
> -  </g>
> +</g> 
> +
> +</defs>
> +
> +<g id="newColor"></g>

Indent this and the <use> tag

::: configure:1
(Diff revision 4)
> +#!/bin/sh

Please do not include this file in your commits

::: old-configure:1
(Diff revision 4)
> +#! /bin/sh

Ditto here, do not include this

Comment 54

[Edouard Oger [:eoger]]

Comment on attachment 8824908 [details]
Deleted pngs

https://reviewboard.mozilla.org/r/103242/#review107986

::: configure:1
(Diff revision 2)
> +#!/bin/sh

Remove this file from your commit

::: old-configure:1
(Diff revision 2)
> +#! /bin/sh

Ditto, this file is not supposed to be here.

Comment 55

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8824776 [details]
Bug 1228478 - Modified the svg such that it can be styled without an external stylesheet. Removed all references of the pngs.

https://reviewboard.mozilla.org/r/103082/#review108034

::: browser/base/content/aboutaccounts/aboutaccounts.xhtml:45
(Diff revision 2)
>          <header>
>            <div id="email"></div>
>          </header>
>  
>          <section>
> -            <div class="graphic graphic-sync-intro"> </div>
> +            <html:object class="fxaSyncIllustration" data="chrome://browser/skin/fxa/sync-illustration.svg#newColor"/>

I think we should make the fragment name more meaningful - eg, something like #aboutaccounts

::: browser/components/customizableui/content/panelUI.inc.xul:134
(Diff revision 2)
>              </vbox>
>              <!-- Sync is ready to Sync but the "tabs" engine isn't enabled-->
>              <hbox id="PanelUI-remotetabs-tabsdisabledpane" pack="center" flex="1">
>                <vbox class="PanelUI-remotetabs-instruction-box">
>                  <hbox pack="center">
> -                  <image class="fxaSyncIllustration" alt=""/>
> +                    <html:object class="fxaSyncIllustration" data="chrome://browser/skin/fxa/sync-illustration.svg#newColor"/>

#newcolor here seems wrong - isn't this where we'd use the original SVG color?

::: browser/themes/shared/fxa/sync-illustration.svg:4
(Diff revision 2)
> -<?xml version="1.0" encoding="utf-8"?>
> -<!-- This Source Code Form is subject to the terms of the Mozilla Public
> -   - License, v. 2.0. If a copy of the MPL was not distributed with this
> -   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +<?xml version="1.0" encoding="UTF-8" standalone="no"?>
> +<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
> +<svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMidYMid" width="320" height="280" viewBox="0 0 320 280" xmlns:xlink="http://www.w3.org/1999/xlink" >
> + 	<style>

we still need a licence in this file, and this line is indented incorrectly.

::: browser/themes/shared/fxa/sync-illustration.svg:25
(Diff revision 2)
>      <path d="M182.000,54.000 L182.000,52.000 L190.000,52.000 L190.000,54.000 L182.000,54.000 ZM170.000,52.000 L178.000,52.000 L178.000,54.000 L170.000,54.000 L170.000,52.000 ZM168.488,60.071 L161.417,53.000 L168.488,45.929 L169.902,47.343 L164.245,53.000 L169.902,58.657 L168.488,60.071 Z"/>
>      <path d="M297.688,276.000 L102.312,276.000 C97.721,276.000 94.000,272.279 94.000,267.688 L94.000,260.000 L306.000,260.000 L306.000,267.688 C306.000,272.279 302.279,276.000 297.688,276.000 ZM117.906,150.312 C117.906,145.721 121.628,142.000 126.218,142.000 L273.688,142.000 C278.279,142.000 282.000,145.721 282.000,150.312 L282.000,256.000 L117.906,256.000 L117.906,150.312 ZM132.000,242.000 L270.000,242.000 L270.000,156.000 L132.000,156.000 L132.000,242.000 Z"/>
>      <path d="M307.074,115.969 L206.926,115.969 C203.101,115.969 200.000,112.868 200.000,109.042 L200.000,38.926 C200.000,35.101 203.101,32.000 206.926,32.000 L307.074,32.000 C310.899,32.000 314.000,35.101 314.000,38.926 L314.000,109.042 C314.000,112.868 310.899,115.969 307.074,115.969 ZM210.000,65.875 C210.000,64.770 209.105,63.875 208.000,63.875 C206.895,63.875 206.000,64.770 206.000,65.875 L206.000,82.000 C206.000,83.105 206.895,84.000 208.000,84.000 C209.105,84.000 210.000,83.105 210.000,82.000 L210.000,65.875 ZM302.000,42.000 L216.000,42.000 L216.000,106.000 L302.000,106.000 L302.000,42.000 Z"/>
>      <path d="M65.844,240.000 L26.156,240.000 C23.861,240.000 22.000,238.139 22.000,235.844 L22.000,162.156 C22.000,159.861 23.861,158.000 26.156,158.000 L65.844,158.000 C68.139,158.000 70.000,159.861 70.000,162.156 L70.000,235.844 C70.000,238.139 68.139,240.000 65.844,240.000 ZM46.000,236.000 C48.287,236.000 50.141,234.195 50.141,231.969 C50.141,229.742 48.287,227.938 46.000,227.938 C43.713,227.938 41.859,229.742 41.859,231.969 C41.859,234.195 43.713,236.000 46.000,236.000 ZM66.000,168.000 L26.000,168.000 L26.000,224.000 L66.000,224.000 L66.000,168.000 Z"/>
>      <path d="M171.906,86.156 C171.906,102.329 159.026,115.469 143.017,115.797 L143.039,115.955 L28.850,115.955 L28.869,115.797 C12.872,115.475 -0.000,102.333 -0.000,86.156 C-0.000,71.661 10.336,59.603 23.994,57.019 C23.620,55.457 23.401,53.834 23.401,52.156 C23.401,40.714 32.606,31.438 43.962,31.438 C47.561,31.438 50.941,32.375 53.884,34.012 C53.883,33.930 53.878,33.848 53.878,33.766 C53.878,17.137 67.301,3.656 83.858,3.656 C97.763,3.656 109.453,13.164 112.843,26.059 C116.677,23.334 121.343,21.719 126.393,21.719 C139.394,21.719 149.933,32.331 149.933,45.422 C149.933,49.572 148.868,53.468 147.007,56.861 C161.114,59.082 171.906,71.351 171.906,86.156 Z"/>
> -  </g>
> +</g> 

trailing space here

Comment 56

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8824908 [details]
Deleted pngs

https://reviewboard.mozilla.org/r/103242/#review108036

Comment 57

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8824908 [details]
Deleted pngs

https://reviewboard.mozilla.org/r/103242/#review108038

Comment 58

[Mark Hammond [:markh] [:mhammond]]

Looks good apart from Edouard and my comments - thanks!

Comment 59

[Dorel Barbu]

Comment on attachment 8824776 [details]
Bug 1228478 - Modified the svg such that it can be styled without an external stylesheet. Removed all references of the pngs.

https://reviewboard.mozilla.org/r/103082/#review108034

> I think we should make the fragment name more meaningful - eg, something like #aboutaccounts

I renamed the class: aboutaccounts. Also, I have renamed the graphic-sync-intro style in main.css to be consistent. I had forgotten about this before pushing the changes. Thank you.

Comment 60

[Dorel Barbu]

Attachment: Bug 1228478 - Resolved issues

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

Comment 61

[Dorel Barbu]

Attachment: Bug 1228478 - Replaced the use of sync-illustration.png and [e-mail] with sync-illustration.svg

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

Comment 62

[Dorel Barbu]

Comment on attachment 8830267 [details]
Bug 1228478 - Replaced the use of sync-illustration.png and [e-mail] with sync-illustration.svg

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

Comment 63

[Dorel Barbu]

Comment on attachment 8830267 [details]
Bug 1228478 - Replaced the use of sync-illustration.png and [e-mail] with sync-illustration.svg

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

Comment 64

[Dorel Barbu]

I modified and I hope that it is ok now. How does it look now?

Comment 65

[Edouard Oger [:eoger]]

Comment on attachment 8830267 [details]
Bug 1228478 - Replaced the use of sync-illustration.png and [e-mail] with sync-illustration.svg

https://reviewboard.mozilla.org/r/107148/#review108306

Great! Apart from the fragment name issue, I think we're almost there.
Please also rename your commit to something along the lines of "Bug 1228478 - Replace sync-illustration.png with sync-illustration.svg. r?markh, eoger". Remember what's important in a commit message is the end result, not the way you implemented it.

::: browser/base/content/aboutaccounts/aboutaccounts.xhtml:45
(Diff revision 3)
>          <header>
>            <div id="email"></div>
>          </header>
>  
>          <section>
> -            <div class="graphic graphic-sync-intro"> </div>
> +            <object class="aboutaccounts" data="chrome://browser/skin/fxa/sync-illustration.svg#newColor"/>

The class name was fine, it was the fragment name (newColor) that needed changing.

::: browser/themes/shared/fxa/sync-illustration.svg:30
(Diff revision 3)
> -    <path d="M65.844,240.000 L26.156,240.000 C23.861,240.000 22.000,238.139 22.000,235.844 L22.000,162.156 C22.000,159.861 23.861,158.000 26.156,158.000 L65.844,158.000 C68.139,158.000 70.000,159.861 70.000,162.156 L70.000,235.844 C70.000,238.139 68.139,240.000 65.844,240.000 ZM46.000,236.000 C48.287,236.000 50.141,234.195 50.141,231.969 C50.141,229.742 48.287,227.938 46.000,227.938 C43.713,227.938 41.859,229.742 41.859,231.969 C41.859,234.195 43.713,236.000 46.000,236.000 ZM66.000,168.000 L26.000,168.000 L26.000,224.000 L66.000,224.000 L66.000,168.000 Z"/>
> +        <path d="M65.844,240.000 L26.156,240.000 C23.861,240.000 22.000,238.139 22.000,235.844 L22.000,162.156 C22.000,159.861 23.861,158.000 26.156,158.000 L65.844,158.000 C68.139,158.000 70.000,159.861 70.000,162.156 L70.000,235.844 C70.000,238.139 68.139,240.000 65.844,240.000 ZM46.000,236.000 C48.287,236.000 50.141,234.195 50.141,231.969 C50.141,229.742 48.287,227.938 46.000,227.938 C43.713,227.938 41.859,229.742 41.859,231.969 C41.859,234.195 43.713,236.000 46.000,236.000 ZM66.000,168.000 L26.000,168.000 L26.000,224.000 L66.000,224.000 L66.000,168.000 Z"/>
> -    <path d="M171.906,86.156 C171.906,102.329 159.026,115.469 143.017,115.797 L143.039,115.955 L28.850,115.955 L28.869,115.797 C12.872,115.475 -0.000,102.333 -0.000,86.156 C-0.000,71.661 10.336,59.603 23.994,57.019 C23.620,55.457 23.401,53.834 23.401,52.156 C23.401,40.714 32.606,31.438 43.962,31.438 C47.561,31.438 50.941,32.375 53.884,34.012 C53.883,33.930 53.878,33.848 53.878,33.766 C53.878,17.137 67.301,3.656 83.858,3.656 C97.763,3.656 109.453,13.164 112.843,26.059 C116.677,23.334 121.343,21.719 126.393,21.719 C139.394,21.719 149.933,32.331 149.933,45.422 C149.933,49.572 148.868,53.468 147.007,56.861 C161.114,59.082 171.906,71.351 171.906,86.156 Z"/>
> +        <path d="M171.906,86.156 C171.906,102.329 159.026,115.469 143.017,115.797 L143.039,115.955 L28.850,115.955 L28.869,115.797 C12.872,115.475 -0.000,102.333 -0.000,86.156 C-0.000,71.661 10.336,59.603 23.994,57.019 C23.620,55.457 23.401,53.834 23.401,52.156 C23.401,40.714 32.606,31.438 43.962,31.438 C47.561,31.438 50.941,32.375 53.884,34.012 C53.883,33.930 53.878,33.848 53.878,33.766 C53.878,17.137 67.301,3.656 83.858,3.656 C97.763,3.656 109.453,13.164 112.843,26.059 C116.677,23.334 121.343,21.719 126.393,21.719 C139.394,21.719 149.933,32.331 149.933,45.422 C149.933,49.572 148.868,53.468 147.007,56.861 C161.114,59.082 171.906,71.351 171.906,86.156 Z"/>
> -  </g>
> +    </g>
> +</defs>
> +<g id="newColor"></g>
> +<g id="oldColor"></g>

This is not used anywhere, please delete it.

Comment 66

[Dorel Barbu]

Comment on attachment 8830267 [details]
Bug 1228478 - Replaced the use of sync-illustration.png and [e-mail] with sync-illustration.svg

https://reviewboard.mozilla.org/r/107148/#review108306

Thank you again

> The class name was fine, it was the fragment name (newColor) that needed changing.

Oh...I see what you mean. It might be a silly blunder buy I hadn't realised that this was the fragment name. Sorry :(

> This is not used anywhere, please delete it.

This was some garbage I forgot about.

Comment 67

[Dorel Barbu]

Comment on attachment 8830267 [details]
Bug 1228478 - Replaced the use of sync-illustration.png and [e-mail] with sync-illustration.svg

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/107148/diff/3-4/

Comment 68

[Edouard Oger [:eoger]]

Comment on attachment 8830267 [details]
Bug 1228478 - Replaced the use of sync-illustration.png and [e-mail] with sync-illustration.svg

https://reviewboard.mozilla.org/r/107148/#review108352

This looks good to me, let's wait on markh's final review (he's out of the office today) before landing this. Good work!

Comment 69

[Dorel Barbu]

(In reply to Edouard Oger [:eoger] from comment #68)
> Comment on attachment 8830267 [details]
> Bug 1228478 - Replaced the use of sync-illustration.png and
> [e-mail] with sync-illustration.svg
> 
> https://reviewboard.mozilla.org/r/107148/#review108352
> 
> This looks good to me, let's wait on markh's final review (he's out of the
> office today) before landing this. Good work!

Thank you very much! I'll be waiting for Mark's review. If everything is fine I'll start working on a new bug. Do you have some suggestions?

Comment 70

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8830267 [details]
Bug 1228478 - Replaced the use of sync-illustration.png and [e-mail] with sync-illustration.svg

https://reviewboard.mozilla.org/r/107148/#review109182

This looks great, thanks very much for your persistence :)

Comment 71

[Dorel Barbu]

(In reply to Mark Hammond [:markh] from comment #70)
> Comment on attachment 8830267 [details]
> Bug 1228478 - Replaced the use of sync-illustration.png and
> [e-mail] with sync-illustration.svg
> 
> https://reviewboard.mozilla.org/r/107148/#review109182
> 
> This looks great, thanks very much for your persistence :)

Thank you, Mark! You and Edouard were of great help. What should I do next? Is this bug solved now? Do I have to do anything else here?

Comment 72

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4f2d1a062ed1
Replaced the use of sync-illustration.png and [e-mail] with sync-illustration.svg r=eoger,markh

Comment 73

[Edouard Oger [:eoger]]

Mark ran a series of tests (https://treeherder.mozilla.org/#/jobs?repo=try&revision=d047139e87f7https://treeherder.mozilla.org/#/jobs?repo=try&revision=d047139e87f7) against your patch, looks like everything went OK.
I submitted your patch using autoland: if everything goes right your patch will be on mozilla-central soon! (and the bug will be considered as solved)

Comment 74

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Sorry, I had to back it out for mass test failures:

https://hg.mozilla.org/integration/autoland/rev/e5654fb71a64927f1617fa3ec3a2f71aaf487cae

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=4f2d1a062ed1b6d2a3e3cc3db5456aefc6fb9d0a
Failure log example: https://treeherder.mozilla.org/logviewer.html#?job_id=73017234&repo=autoland

ask 2017-01-30T16:08:30.376276Z] 16:08:30     INFO - TEST-START | dom/events/test/test_bug422132.html
[task 2017-01-30T16:08:32.072430Z] 16:08:32     INFO - TEST-INFO | started process screentopng
[task 2017-01-30T16:08:33.531146Z] 16:08:33     INFO - TEST-INFO | screentopng: exit 0
[task 2017-01-30T16:08:33.534810Z] 16:08:33     INFO - Buffered messages logged at 16:08:31
[task 2017-01-30T16:08:33.536593Z] 16:08:33     INFO - must wait for load
[task 2017-01-30T16:08:33.538343Z] 16:08:33     INFO - Buffered messages finished
[task 2017-01-30T16:08:33.540916Z] 16:08:33     INFO - TEST-UNEXPECTED-FAIL | dom/events/test/test_bug422132.html | not scrolled to right by 0.5px delta value with pending 0.5px delta - got 0, expected 1
[task 2017-01-30T16:08:33.542426Z] 16:08:33     INFO -     SimpleTest.is@SimpleTest/SimpleTest.js:271:5
[task 2017-01-30T16:08:33.543893Z] 16:08:33     INFO -     check@dom/events/test/test_bug422132.html:65:9
[task 2017-01-30T16:08:33.546656Z] 16:08:33     INFO -     nextTest/<@dom/events/test/test_bug422132.html:107:9

Please fix this issue and submit the updated patch. Thank you.

Comment 75

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Also keep the indentation. At the moment, some lines in sync.xul got one or two columns more indentation.

Comment 76

[Dorel Barbu]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #74)
> Sorry, I had to back it out for mass test failures:
> 
> https://hg.mozilla.org/integration/autoland/rev/
> e5654fb71a64927f1617fa3ec3a2f71aaf487cae
> 
> Push with failures:
> https://treeherder.mozilla.org/#/
> jobs?repo=autoland&revision=4f2d1a062ed1b6d2a3e3cc3db5456aefc6fb9d0a
> Failure log example:
> https://treeherder.mozilla.org/logviewer.html#?job_id=73017234&repo=autoland
> 
> ask 2017-01-30T16:08:30.376276Z] 16:08:30     INFO - TEST-START |
> dom/events/test/test_bug422132.html
> [task 2017-01-30T16:08:32.072430Z] 16:08:32     INFO - TEST-INFO | started
> process screentopng
> [task 2017-01-30T16:08:33.531146Z] 16:08:33     INFO - TEST-INFO |
> screentopng: exit 0
> [task 2017-01-30T16:08:33.534810Z] 16:08:33     INFO - Buffered messages
> logged at 16:08:31
> [task 2017-01-30T16:08:33.536593Z] 16:08:33     INFO - must wait for load
> [task 2017-01-30T16:08:33.538343Z] 16:08:33     INFO - Buffered messages
> finished
> [task 2017-01-30T16:08:33.540916Z] 16:08:33     INFO - TEST-UNEXPECTED-FAIL
> | dom/events/test/test_bug422132.html | not scrolled to right by 0.5px delta
> value with pending 0.5px delta - got 0, expected 1
> [task 2017-01-30T16:08:33.542426Z] 16:08:33     INFO -    
> SimpleTest.is@SimpleTest/SimpleTest.js:271:5
> [task 2017-01-30T16:08:33.543893Z] 16:08:33     INFO -    
> check@dom/events/test/test_bug422132.html:65:9
> [task 2017-01-30T16:08:33.546656Z] 16:08:33     INFO -    
> nextTest/<@dom/events/test/test_bug422132.html:107:9
> 
> Please fix this issue and submit the updated patch. Thank you.

Hello, Sebastian! Thanks for pointing it out. I will gladly try to solve the problems, unfortunately I don't know how. I don't know where to start, and I am a bit confused. I have some questions:

1. You gave me an example of failure. If I understood corectly, the problem is that:

/test/test_bug422132.html | not scrolled to right by 0.5px delta value with pending 0.5px delta - got 0, expected 1

The problem is that I don't know how to replicate the problem on my laptop, I don't understand what it means. Sorry if the question is dumb, but how can I replicate this behaviour and see that there is indeed a problem?  I really need help interpreting this log. 

2. Let's say that I found a way to solve the problem stated at 1. How can I check before posting the patch update, that the problem is indeed gone?

Could you please help me out a little?

Comment 77

[Dorel Barbu]

After a little research I ran: ./mach mochitest dom/events/test/test_bug422132.html, which I think is the test above. The result is:

0 INFO TEST-START | Shutdown
1 INFO Passed:  4
2 INFO Failed:  0
3 INFO Todo:    0
4 INFO Mode:    e10s
5 INFO SimpleTest FINISHED
Buffered messages finished
SUITE-END | took 70s

So apparently it passes? I am missing something, but what?

Comment 78

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

(In reply to Dorel Barbu from comment #76)
> 1. You gave me an example of failure. If I understood corectly, the problem
> is that:
> 
> /test/test_bug422132.html | not scrolled to right by 0.5px delta value with
> pending 0.5px delta - got 0, expected 1
Thanks for already figuring out how to run the test.

> The problem is that I don't know how to replicate the problem on my laptop,
> I don't understand what it means. Sorry if the question is dumb, but how can
> I replicate this behaviour and see that there is indeed a problem?  I really
> need help interpreting this log. 
Some of the failed tests mention that there are 2 assertions. These must be new. One of the mentors should help you to get them.

> 2. Let's say that I found a way to solve the problem stated at 1. How can I
> check before posting the patch update, that the problem is indeed gone?
Push you updated patch to mozreview/reviewboard and one of the mentors will trigger a Try run. The previous one already showed the failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d047139e87f7

> Could you please help me out a little?
I don't have an exact idea what's causing it because even a JavaScript reftest fails.

Comment 79

[Dorel Barbu]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #78)
> (In reply to Dorel Barbu from comment #76)
> > 1. You gave me an example of failure. If I understood corectly, the problem
> > is that:
> > 
> > /test/test_bug422132.html | not scrolled to right by 0.5px delta value with
> > pending 0.5px delta - got 0, expected 1
> Thanks for already figuring out how to run the test.
> 
> > The problem is that I don't know how to replicate the problem on my laptop,
> > I don't understand what it means. Sorry if the question is dumb, but how can
> > I replicate this behaviour and see that there is indeed a problem?  I really
> > need help interpreting this log. 
> Some of the failed tests mention that there are 2 assertions. These must be
> new. One of the mentors should help you to get them.
> 
> > 2. Let's say that I found a way to solve the problem stated at 1. How can I
> > check before posting the patch update, that the problem is indeed gone?
> Push you updated patch to mozreview/reviewboard and one of the mentors will
> trigger a Try run. The previous one already showed the failures:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=d047139e87f7
> 
> > Could you please help me out a little?
> I don't have an exact idea what's causing it because even a JavaScript
> reftest fails.

Ok, this gives me some idea about the workflow. The next question is: in comment #77 I mentioned that I ran on my machine a test that initially failed. I haven't touched anything, I haven't made any sort of modifications to the source code. I double checked that using hg status. Yet, on my machine it pass, and on server it fails. I don't know how to track what's causing the problem since on my machine it works fine. Any suggestions?

By the way, the next patch will be correctly indented. Sorry about that

Comment 80

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Sorry, I don't know what changed. Running the debug build from the Try run prints the following message to the console:

[Parent 2794] WARNING: Failed to open external DTD: publicId "-//W3C//DTD SVG 1.1//EN" systemId "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" base "jar:file:///home/user/fx-temp/browser/omni.ja!/chrome/browser/skin/classic/browser/fxa/sync-illustration.svg#blueFill" URL "resource://gre/res/dtd/svg11.dtd": file /home/worker/workspace/build/src/parser/htmlparser/nsExpatDriver.cpp, line 702

Mark, can you help Dorel debug the e10s failures, please?

Comment 81

[Dorel Barbu]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #80)
> Sorry, I don't know what changed. Running the debug build from the Try run
> prints the following message to the console:
> 
> [Parent 2794] WARNING: Failed to open external DTD: publicId "-//W3C//DTD
> SVG 1.1//EN" systemId "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd"
> base
> "jar:file:///home/user/fx-temp/browser/omni.ja!/chrome/browser/skin/classic/
> browser/fxa/sync-illustration.svg#blueFill" URL
> "resource://gre/res/dtd/svg11.dtd": file
> /home/worker/workspace/build/src/parser/htmlparser/nsExpatDriver.cpp, line
> 702
> 
> Mark, can you help Dorel debug the e10s failures, please?

OK, then I'll wait to see if Mark has any clue about it. Thank you for making time to help me!

Comment 82

[Edouard Oger [:eoger]]

Comment on attachment 8830267 [details]
Bug 1228478 - Replaced the use of sync-illustration.png and [e-mail] with sync-illustration.svg

https://reviewboard.mozilla.org/r/107146/#review109426

::: browser/themes/shared/fxa/sync-illustration.svg:1
(Diff revision 4)
> -<?xml version="1.0" encoding="utf-8"?>
> +<?xml version="1.0" encoding="UTF-8" standalone="no"?>

Nitpick: the line change wasn't needed, please revert it back.

::: browser/themes/shared/fxa/sync-illustration.svg:5
(Diff revision 4)
> -<?xml version="1.0" encoding="utf-8"?>
> +<?xml version="1.0" encoding="UTF-8" standalone="no"?>
>  <!-- This Source Code Form is subject to the terms of the Mozilla Public
>     - License, v. 2.0. If a copy of the MPL was not distributed with this
>     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> -<svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMidYMid" width="320" height="280" viewBox="0 0 320 280">
> +<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">

This is probably what triggers the failed tests. Please remove the DOCTYPE declaration.

Comment 83

[Mark Hammond [:markh] [:mhammond]]

Thanks Edouard!

Comment 84

[Dorel Barbu]

Comment on attachment 8830267 [details]
Bug 1228478 - Replaced the use of sync-illustration.png and [e-mail] with sync-illustration.svg

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/107148/diff/4-5/

Comment 85

[Dorel Barbu]

Hello. Thanks again for helping me out. I've updated the patch. I modified those lines and I fixed some indentation issues (hopefully, all of them). Could you please look into it and trigger a Try run?

Comment 86

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Pushed to Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3abf61cf316

Comment 87

[Dorel Barbu]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #86)
> Pushed to Try:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3abf61cf316

Thank you. Can't wait to see the final conclusion.

Comment 88

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Still fails. From the log:

[task 2017-01-31T10:00:45.858500Z] 10:00:45     INFO - JavaScript error: chrome://browser/content/tabbrowser.xml, line 1791: TypeError: aBrowser.webProgress is undefined
[task 2017-01-31T10:00:46.499966Z] 10:00:46     INFO - ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to /tmp/tmpq8a8wq.mozrunner/runreftest_leaks_tab_pid1072.log
[task 2017-01-31T10:00:46.629117Z] 10:00:46     INFO - [Parent 1018] ###!!! ASSERTION: Wrong event target!: 'this == fl->GetTabChildGlobalAsEventTarget()', file /home/worker/workspace/build/src/dom/base/nsInProcessTabChildGlobal.cpp, line 265
[task 2017-01-31T10:01:31.692318Z] 10:01:31     INFO - #01: mozilla::EventTargetChainItem::GetEventTargetParent [dom/events/EventDispatcher.cpp:211]
[task 2017-01-31T10:01:31.692493Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.693867Z] 10:01:31     INFO - #02: mozilla::EventDispatcher::Dispatch [dom/events/EventDispatcher.cpp:748]
[task 2017-01-31T10:01:31.693976Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.694888Z] 10:01:31     INFO - #03: mozilla::EventDispatcher::DispatchDOMEvent [dom/events/EventDispatcher.cpp:889]
[task 2017-01-31T10:01:31.695410Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.696614Z] 10:01:31     INFO - #04: mozilla::DOMEventTargetHelper::DispatchEvent [dom/events/DOMEventTargetHelper.cpp:281]
[task 2017-01-31T10:01:31.697084Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.698283Z] 10:01:31     INFO - #05: mozilla::DOMEventTargetHelper::DispatchTrustedEvent [dom/events/DOMEventTargetHelper.cpp:301]
[task 2017-01-31T10:01:31.698870Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.700070Z] 10:01:31     INFO - #06: mozilla::DOMEventTargetHelper::DispatchTrustedEvent [dom/events/DOMEventTargetHelper.cpp:291]
[task 2017-01-31T10:01:31.700653Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.701815Z] 10:01:31     INFO - #07: nsInProcessTabChildGlobal::FireUnloadEvent [xpcom/string/nsTSubstring.h:95]
[task 2017-01-31T10:01:31.702393Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.703589Z] 10:01:31     INFO - #08: nsFrameLoader::DestroyDocShell [dom/base/nsFrameLoader.cpp:2092]
[task 2017-01-31T10:01:31.704171Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.705411Z] 10:01:31     INFO - #09: nsFrameLoaderDestroyRunnable::Run [dom/base/nsFrameLoader.cpp:2043]
[task 2017-01-31T10:01:31.705959Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.707240Z] 10:01:31     INFO - #10: nsDocument::MaybeInitializeFinalizeFrameLoaders [dom/base/nsDocument.cpp:6968]
[task 2017-01-31T10:01:31.707830Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.709184Z] 10:01:31     INFO - #11: mozilla::dom::XULDocument::DoneWalking [xpcom/glue/nsTObserverArray.h:302]
[task 2017-01-31T10:01:31.709787Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.711179Z] 10:01:31     INFO - #12: mozilla::dom::XULDocument::ResumeWalk [dom/xul/XULDocument.cpp:2964]
[task 2017-01-31T10:01:31.712164Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.713486Z] 10:01:31     INFO - #13: mozilla::dom::XULDocument::EndLoad [dom/xul/XULDocument.cpp:548]
[task 2017-01-31T10:01:31.714592Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.715618Z] 10:01:31     INFO - #14: XULContentSinkImpl::DidBuildModel [dom/xul/nsXULContentSink.cpp:229]
[task 2017-01-31T10:01:31.716627Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.717787Z] 10:01:31     INFO - #15: nsParser::DidBuildModel [parser/htmlparser/nsParser.cpp:901]
[task 2017-01-31T10:01:31.718805Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.720183Z] 10:01:31     INFO - #16: nsParser::ResumeParse [parser/htmlparser/nsParser.cpp:1507]
[task 2017-01-31T10:01:31.721355Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.722772Z] 10:01:31     INFO - #17: nsParser::OnStopRequest [parser/htmlparser/nsParser.cpp:1881]
[task 2017-01-31T10:01:31.723657Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.725077Z] 10:01:31     INFO - #18: nsJARChannel::OnStopRequest [modules/libjar/nsJARChannel.cpp:1019]
[task 2017-01-31T10:01:31.725969Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.727385Z] 10:01:31     INFO - #19: nsInputStreamPump::OnStateStop [netwerk/base/nsInputStreamPump.cpp:715]
[task 2017-01-31T10:01:31.728278Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.729661Z] 10:01:31     INFO - #20: nsInputStreamPump::OnInputStreamReady [netwerk/base/nsInputStreamPump.cpp:433]
[task 2017-01-31T10:01:31.730561Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.731769Z] 10:01:31     INFO - #21: nsInputStreamReadyEvent::Run [xpcom/glue/nsCOMPtr.h:600]
[task 2017-01-31T10:01:31.732866Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.734056Z] 10:01:31     INFO - #22: nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:1240]
[task 2017-01-31T10:01:31.734985Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.736384Z] 10:01:31     INFO - #23: NS_ProcessNextEvent [xpcom/glue/nsThreadUtils.cpp:390]
[task 2017-01-31T10:01:31.737270Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.738511Z] 10:01:31     INFO - #24: mozilla::ipc::MessagePump::Run [ipc/glue/MessagePump.cpp:97]
[task 2017-01-31T10:01:31.739580Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.740778Z] 10:01:31     INFO - #25: MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:239]
[task 2017-01-31T10:01:31.742057Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.743093Z] 10:01:31     INFO - #26: MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:502]
[task 2017-01-31T10:01:31.744202Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.745409Z] 10:01:31     INFO - #27: nsBaseAppShell::Run [widget/nsBaseAppShell.cpp:158]
[task 2017-01-31T10:01:31.746506Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.747527Z] 10:01:31     INFO - #28: nsAppStartup::Run [toolkit/components/startup/nsAppStartup.cpp:284]
[task 2017-01-31T10:01:31.748668Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.750001Z] 10:01:31     INFO - #29: XREMain::XRE_mainRun [toolkit/xre/nsAppRunner.cpp:4495]
[task 2017-01-31T10:01:31.750895Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.752256Z] 10:01:31     INFO - #30: XREMain::XRE_main [toolkit/xre/nsAppRunner.cpp:4623]
[task 2017-01-31T10:01:31.753174Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.754576Z] 10:01:31     INFO - #31: XRE_main [toolkit/xre/nsAppRunner.cpp:4714]
[task 2017-01-31T10:01:31.755466Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.799433Z] 10:01:31     INFO - #32: do_main [browser/app/nsBrowserApp.cpp:261]
[task 2017-01-31T10:01:31.799589Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.800663Z] 10:01:31     INFO - #33: main [browser/app/nsBrowserApp.cpp:452]
[task 2017-01-31T10:01:31.801112Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.802560Z] 10:01:31     INFO - #34: libc.so.6 + 0x20830
[task 2017-01-31T10:01:31.803158Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.804335Z] 10:01:31     INFO - #35: _start
[task 2017-01-31T10:01:31.804791Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.806006Z] 10:01:31     INFO - [Parent 1018] ###!!! ASSERTION: Wrong message manager!: 'fl->mMessageManager == mChromeMessageManager', file /home/worker/workspace/build/src/dom/base/nsInProcessTabChildGlobal.cpp, line 267
[task 2017-01-31T10:01:31.807278Z] 10:01:31     INFO - #01: mozilla::EventTargetChainItem::GetEventTargetParent [dom/events/EventDispatcher.cpp:211]
[task 2017-01-31T10:01:31.807802Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.808856Z] 10:01:31     INFO - #02: mozilla::EventDispatcher::Dispatch [dom/events/EventDispatcher.cpp:748]
[task 2017-01-31T10:01:31.809451Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.810602Z] 10:01:31     INFO - #03: mozilla::EventDispatcher::DispatchDOMEvent [dom/events/EventDispatcher.cpp:889]
[task 2017-01-31T10:01:31.811175Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.812546Z] 10:01:31     INFO - #04: mozilla::DOMEventTargetHelper::DispatchEvent [dom/events/DOMEventTargetHelper.cpp:281]
[task 2017-01-31T10:01:31.813178Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.814621Z] 10:01:31     INFO - #05: mozilla::DOMEventTargetHelper::DispatchTrustedEvent [dom/events/DOMEventTargetHelper.cpp:301]
[task 2017-01-31T10:01:31.815141Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.816275Z] 10:01:31     INFO - #06: mozilla::DOMEventTargetHelper::DispatchTrustedEvent [dom/events/DOMEventTargetHelper.cpp:291]
[task 2017-01-31T10:01:31.816865Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.818183Z] 10:01:31     INFO - #07: nsInProcessTabChildGlobal::FireUnloadEvent [xpcom/string/nsTSubstring.h:95]
[task 2017-01-31T10:01:31.818748Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.820129Z] 10:01:31     INFO - #08: nsFrameLoader::DestroyDocShell [dom/base/nsFrameLoader.cpp:2092]
[task 2017-01-31T10:01:31.821179Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.822655Z] 10:01:31     INFO - #09: nsFrameLoaderDestroyRunnable::Run [dom/base/nsFrameLoader.cpp:2043]
[task 2017-01-31T10:01:31.823609Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.824869Z] 10:01:31     INFO - #10: nsDocument::MaybeInitializeFinalizeFrameLoaders [dom/base/nsDocument.cpp:6968]
[task 2017-01-31T10:01:31.826034Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.827256Z] 10:01:31     INFO - #11: mozilla::dom::XULDocument::DoneWalking [xpcom/glue/nsTObserverArray.h:302]
[task 2017-01-31T10:01:31.828196Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.829611Z] 10:01:31     INFO - #12: mozilla::dom::XULDocument::ResumeWalk [dom/xul/XULDocument.cpp:2964]
[task 2017-01-31T10:01:31.830728Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.831983Z] 10:01:31     INFO - #13: mozilla::dom::XULDocument::EndLoad [dom/xul/XULDocument.cpp:548]
[task 2017-01-31T10:01:31.832936Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.834206Z] 10:01:31     INFO - #14: XULContentSinkImpl::DidBuildModel [dom/xul/nsXULContentSink.cpp:229]
[task 2017-01-31T10:01:31.835136Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.836331Z] 10:01:31     INFO - #15: nsParser::DidBuildModel [parser/htmlparser/nsParser.cpp:901]
[task 2017-01-31T10:01:31.837322Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.838509Z] 10:01:31     INFO - #16: nsParser::ResumeParse [parser/htmlparser/nsParser.cpp:1507]
[task 2017-01-31T10:01:31.839538Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.840996Z] 10:01:31     INFO - #17: nsParser::OnStopRequest [parser/htmlparser/nsParser.cpp:1881]
[task 2017-01-31T10:01:31.841928Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.843112Z] 10:01:31     INFO - #18: nsJARChannel::OnStopRequest [modules/libjar/nsJARChannel.cpp:1019]
[task 2017-01-31T10:01:31.844237Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.845329Z] 10:01:31     INFO - #19: nsInputStreamPump::OnStateStop [netwerk/base/nsInputStreamPump.cpp:715]
[task 2017-01-31T10:01:31.846368Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.847930Z] 10:01:31     INFO - #20: nsInputStreamPump::OnInputStreamReady [netwerk/base/nsInputStreamPump.cpp:433]
[task 2017-01-31T10:01:31.848800Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.850195Z] 10:01:31     INFO - #21: nsInputStreamReadyEvent::Run [xpcom/glue/nsCOMPtr.h:600]
[task 2017-01-31T10:01:31.851297Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.852370Z] 10:01:31     INFO - #22: nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:1240]
[task 2017-01-31T10:01:31.853420Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.854546Z] 10:01:31     INFO - #23: NS_ProcessNextEvent [xpcom/glue/nsThreadUtils.cpp:390]
[task 2017-01-31T10:01:31.855715Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.856768Z] 10:01:31     INFO - #24: mozilla::ipc::MessagePump::Run [ipc/glue/MessagePump.cpp:97]
[task 2017-01-31T10:01:31.858068Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.859276Z] 10:01:31     INFO - #25: MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:239]
[task 2017-01-31T10:01:31.860278Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.861539Z] 10:01:31     INFO - #26: MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:502]
[task 2017-01-31T10:01:31.862374Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.863754Z] 10:01:31     INFO - #27: nsBaseAppShell::Run [widget/nsBaseAppShell.cpp:158]
[task 2017-01-31T10:01:31.864747Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.866026Z] 10:01:31     INFO - #28: nsAppStartup::Run [toolkit/components/startup/nsAppStartup.cpp:284]
[task 2017-01-31T10:01:31.867153Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.868204Z] 10:01:31     INFO - #29: XREMain::XRE_mainRun [toolkit/xre/nsAppRunner.cpp:4495]
[task 2017-01-31T10:01:31.869201Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.870433Z] 10:01:31     INFO - #30: XREMain::XRE_main [toolkit/xre/nsAppRunner.cpp:4623]
[task 2017-01-31T10:01:31.871563Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.872824Z] 10:01:31     INFO - #31: XRE_main [toolkit/xre/nsAppRunner.cpp:4714]
[task 2017-01-31T10:01:31.873920Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.875240Z] 10:01:31     INFO - #32: do_main [browser/app/nsBrowserApp.cpp:261]
[task 2017-01-31T10:01:31.876370Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.877170Z] 10:01:31     INFO - #33: main [browser/app/nsBrowserApp.cpp:452]
[task 2017-01-31T10:01:31.877728Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.878533Z] 10:01:31     INFO - #34: libc.so.6 + 0x20830
[task 2017-01-31T10:01:31.879036Z] 10:01:31     INFO - 
[task 2017-01-31T10:01:31.879828Z] 10:01:31     INFO - #35: _start
[task 2017-01-31T10:01:31.880497Z] 10:01:31     INFO - 

html:object isn't used in the codebase at the moment, skinning with CSS and background-image: url(...) seems to be the way to go, see https://dxr.mozilla.org/mozilla-central/search?q=path%3Abrowser%2F+svg&redirect=true
https://dxr.mozilla.org/mozilla-central/source/browser/base/content/abouthome/aboutHome.css#128
https://dxr.mozilla.org/mozilla-central/source/browser/themes/shared/search/search-arrow-go.svg

Comment 89

[Dorel Barbu]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #88)
> Still fails. From the log:
> 
> [task 2017-01-31T10:00:45.858500Z] 10:00:45     INFO - JavaScript error:
> chrome://browser/content/tabbrowser.xml, line 1791: TypeError:
> aBrowser.webProgress is undefined
> [task 2017-01-31T10:00:46.499966Z] 10:00:46     INFO - ###
> XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to
> /tmp/tmpq8a8wq.mozrunner/runreftest_leaks_tab_pid1072.log
> [task 2017-01-31T10:00:46.629117Z] 10:00:46     INFO - [Parent 1018] ###!!!
> ASSERTION: Wrong event target!: 'this ==
> fl->GetTabChildGlobalAsEventTarget()', file
> /home/worker/workspace/build/src/dom/base/nsInProcessTabChildGlobal.cpp,
> line 265
> [task 2017-01-31T10:01:31.692318Z] 10:01:31     INFO - #01:
> mozilla::EventTargetChainItem::GetEventTargetParent
> [dom/events/EventDispatcher.cpp:211]
> [task 2017-01-31T10:01:31.692493Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.693867Z] 10:01:31     INFO - #02:
> mozilla::EventDispatcher::Dispatch [dom/events/EventDispatcher.cpp:748]
> [task 2017-01-31T10:01:31.693976Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.694888Z] 10:01:31     INFO - #03:
> mozilla::EventDispatcher::DispatchDOMEvent
> [dom/events/EventDispatcher.cpp:889]
> [task 2017-01-31T10:01:31.695410Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.696614Z] 10:01:31     INFO - #04:
> mozilla::DOMEventTargetHelper::DispatchEvent
> [dom/events/DOMEventTargetHelper.cpp:281]
> [task 2017-01-31T10:01:31.697084Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.698283Z] 10:01:31     INFO - #05:
> mozilla::DOMEventTargetHelper::DispatchTrustedEvent
> [dom/events/DOMEventTargetHelper.cpp:301]
> [task 2017-01-31T10:01:31.698870Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.700070Z] 10:01:31     INFO - #06:
> mozilla::DOMEventTargetHelper::DispatchTrustedEvent
> [dom/events/DOMEventTargetHelper.cpp:291]
> [task 2017-01-31T10:01:31.700653Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.701815Z] 10:01:31     INFO - #07:
> nsInProcessTabChildGlobal::FireUnloadEvent [xpcom/string/nsTSubstring.h:95]
> [task 2017-01-31T10:01:31.702393Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.703589Z] 10:01:31     INFO - #08:
> nsFrameLoader::DestroyDocShell [dom/base/nsFrameLoader.cpp:2092]
> [task 2017-01-31T10:01:31.704171Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.705411Z] 10:01:31     INFO - #09:
> nsFrameLoaderDestroyRunnable::Run [dom/base/nsFrameLoader.cpp:2043]
> [task 2017-01-31T10:01:31.705959Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.707240Z] 10:01:31     INFO - #10:
> nsDocument::MaybeInitializeFinalizeFrameLoaders
> [dom/base/nsDocument.cpp:6968]
> [task 2017-01-31T10:01:31.707830Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.709184Z] 10:01:31     INFO - #11:
> mozilla::dom::XULDocument::DoneWalking [xpcom/glue/nsTObserverArray.h:302]
> [task 2017-01-31T10:01:31.709787Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.711179Z] 10:01:31     INFO - #12:
> mozilla::dom::XULDocument::ResumeWalk [dom/xul/XULDocument.cpp:2964]
> [task 2017-01-31T10:01:31.712164Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.713486Z] 10:01:31     INFO - #13:
> mozilla::dom::XULDocument::EndLoad [dom/xul/XULDocument.cpp:548]
> [task 2017-01-31T10:01:31.714592Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.715618Z] 10:01:31     INFO - #14:
> XULContentSinkImpl::DidBuildModel [dom/xul/nsXULContentSink.cpp:229]
> [task 2017-01-31T10:01:31.716627Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.717787Z] 10:01:31     INFO - #15:
> nsParser::DidBuildModel [parser/htmlparser/nsParser.cpp:901]
> [task 2017-01-31T10:01:31.718805Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.720183Z] 10:01:31     INFO - #16:
> nsParser::ResumeParse [parser/htmlparser/nsParser.cpp:1507]
> [task 2017-01-31T10:01:31.721355Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.722772Z] 10:01:31     INFO - #17:
> nsParser::OnStopRequest [parser/htmlparser/nsParser.cpp:1881]
> [task 2017-01-31T10:01:31.723657Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.725077Z] 10:01:31     INFO - #18:
> nsJARChannel::OnStopRequest [modules/libjar/nsJARChannel.cpp:1019]
> [task 2017-01-31T10:01:31.725969Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.727385Z] 10:01:31     INFO - #19:
> nsInputStreamPump::OnStateStop [netwerk/base/nsInputStreamPump.cpp:715]
> [task 2017-01-31T10:01:31.728278Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.729661Z] 10:01:31     INFO - #20:
> nsInputStreamPump::OnInputStreamReady
> [netwerk/base/nsInputStreamPump.cpp:433]
> [task 2017-01-31T10:01:31.730561Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.731769Z] 10:01:31     INFO - #21:
> nsInputStreamReadyEvent::Run [xpcom/glue/nsCOMPtr.h:600]
> [task 2017-01-31T10:01:31.732866Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.734056Z] 10:01:31     INFO - #22:
> nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:1240]
> [task 2017-01-31T10:01:31.734985Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.736384Z] 10:01:31     INFO - #23:
> NS_ProcessNextEvent [xpcom/glue/nsThreadUtils.cpp:390]
> [task 2017-01-31T10:01:31.737270Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.738511Z] 10:01:31     INFO - #24:
> mozilla::ipc::MessagePump::Run [ipc/glue/MessagePump.cpp:97]
> [task 2017-01-31T10:01:31.739580Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.740778Z] 10:01:31     INFO - #25:
> MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:239]
> [task 2017-01-31T10:01:31.742057Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.743093Z] 10:01:31     INFO - #26: MessageLoop::Run
> [ipc/chromium/src/base/message_loop.cc:502]
> [task 2017-01-31T10:01:31.744202Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.745409Z] 10:01:31     INFO - #27:
> nsBaseAppShell::Run [widget/nsBaseAppShell.cpp:158]
> [task 2017-01-31T10:01:31.746506Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.747527Z] 10:01:31     INFO - #28:
> nsAppStartup::Run [toolkit/components/startup/nsAppStartup.cpp:284]
> [task 2017-01-31T10:01:31.748668Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.750001Z] 10:01:31     INFO - #29:
> XREMain::XRE_mainRun [toolkit/xre/nsAppRunner.cpp:4495]
> [task 2017-01-31T10:01:31.750895Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.752256Z] 10:01:31     INFO - #30:
> XREMain::XRE_main [toolkit/xre/nsAppRunner.cpp:4623]
> [task 2017-01-31T10:01:31.753174Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.754576Z] 10:01:31     INFO - #31: XRE_main
> [toolkit/xre/nsAppRunner.cpp:4714]
> [task 2017-01-31T10:01:31.755466Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.799433Z] 10:01:31     INFO - #32: do_main
> [browser/app/nsBrowserApp.cpp:261]
> [task 2017-01-31T10:01:31.799589Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.800663Z] 10:01:31     INFO - #33: main
> [browser/app/nsBrowserApp.cpp:452]
> [task 2017-01-31T10:01:31.801112Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.802560Z] 10:01:31     INFO - #34: libc.so.6 +
> 0x20830
> [task 2017-01-31T10:01:31.803158Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.804335Z] 10:01:31     INFO - #35: _start
> [task 2017-01-31T10:01:31.804791Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.806006Z] 10:01:31     INFO - [Parent 1018] ###!!!
> ASSERTION: Wrong message manager!: 'fl->mMessageManager ==
> mChromeMessageManager', file
> /home/worker/workspace/build/src/dom/base/nsInProcessTabChildGlobal.cpp,
> line 267
> [task 2017-01-31T10:01:31.807278Z] 10:01:31     INFO - #01:
> mozilla::EventTargetChainItem::GetEventTargetParent
> [dom/events/EventDispatcher.cpp:211]
> [task 2017-01-31T10:01:31.807802Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.808856Z] 10:01:31     INFO - #02:
> mozilla::EventDispatcher::Dispatch [dom/events/EventDispatcher.cpp:748]
> [task 2017-01-31T10:01:31.809451Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.810602Z] 10:01:31     INFO - #03:
> mozilla::EventDispatcher::DispatchDOMEvent
> [dom/events/EventDispatcher.cpp:889]
> [task 2017-01-31T10:01:31.811175Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.812546Z] 10:01:31     INFO - #04:
> mozilla::DOMEventTargetHelper::DispatchEvent
> [dom/events/DOMEventTargetHelper.cpp:281]
> [task 2017-01-31T10:01:31.813178Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.814621Z] 10:01:31     INFO - #05:
> mozilla::DOMEventTargetHelper::DispatchTrustedEvent
> [dom/events/DOMEventTargetHelper.cpp:301]
> [task 2017-01-31T10:01:31.815141Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.816275Z] 10:01:31     INFO - #06:
> mozilla::DOMEventTargetHelper::DispatchTrustedEvent
> [dom/events/DOMEventTargetHelper.cpp:291]
> [task 2017-01-31T10:01:31.816865Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.818183Z] 10:01:31     INFO - #07:
> nsInProcessTabChildGlobal::FireUnloadEvent [xpcom/string/nsTSubstring.h:95]
> [task 2017-01-31T10:01:31.818748Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.820129Z] 10:01:31     INFO - #08:
> nsFrameLoader::DestroyDocShell [dom/base/nsFrameLoader.cpp:2092]
> [task 2017-01-31T10:01:31.821179Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.822655Z] 10:01:31     INFO - #09:
> nsFrameLoaderDestroyRunnable::Run [dom/base/nsFrameLoader.cpp:2043]
> [task 2017-01-31T10:01:31.823609Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.824869Z] 10:01:31     INFO - #10:
> nsDocument::MaybeInitializeFinalizeFrameLoaders
> [dom/base/nsDocument.cpp:6968]
> [task 2017-01-31T10:01:31.826034Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.827256Z] 10:01:31     INFO - #11:
> mozilla::dom::XULDocument::DoneWalking [xpcom/glue/nsTObserverArray.h:302]
> [task 2017-01-31T10:01:31.828196Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.829611Z] 10:01:31     INFO - #12:
> mozilla::dom::XULDocument::ResumeWalk [dom/xul/XULDocument.cpp:2964]
> [task 2017-01-31T10:01:31.830728Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.831983Z] 10:01:31     INFO - #13:
> mozilla::dom::XULDocument::EndLoad [dom/xul/XULDocument.cpp:548]
> [task 2017-01-31T10:01:31.832936Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.834206Z] 10:01:31     INFO - #14:
> XULContentSinkImpl::DidBuildModel [dom/xul/nsXULContentSink.cpp:229]
> [task 2017-01-31T10:01:31.835136Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.836331Z] 10:01:31     INFO - #15:
> nsParser::DidBuildModel [parser/htmlparser/nsParser.cpp:901]
> [task 2017-01-31T10:01:31.837322Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.838509Z] 10:01:31     INFO - #16:
> nsParser::ResumeParse [parser/htmlparser/nsParser.cpp:1507]
> [task 2017-01-31T10:01:31.839538Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.840996Z] 10:01:31     INFO - #17:
> nsParser::OnStopRequest [parser/htmlparser/nsParser.cpp:1881]
> [task 2017-01-31T10:01:31.841928Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.843112Z] 10:01:31     INFO - #18:
> nsJARChannel::OnStopRequest [modules/libjar/nsJARChannel.cpp:1019]
> [task 2017-01-31T10:01:31.844237Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.845329Z] 10:01:31     INFO - #19:
> nsInputStreamPump::OnStateStop [netwerk/base/nsInputStreamPump.cpp:715]
> [task 2017-01-31T10:01:31.846368Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.847930Z] 10:01:31     INFO - #20:
> nsInputStreamPump::OnInputStreamReady
> [netwerk/base/nsInputStreamPump.cpp:433]
> [task 2017-01-31T10:01:31.848800Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.850195Z] 10:01:31     INFO - #21:
> nsInputStreamReadyEvent::Run [xpcom/glue/nsCOMPtr.h:600]
> [task 2017-01-31T10:01:31.851297Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.852370Z] 10:01:31     INFO - #22:
> nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:1240]
> [task 2017-01-31T10:01:31.853420Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.854546Z] 10:01:31     INFO - #23:
> NS_ProcessNextEvent [xpcom/glue/nsThreadUtils.cpp:390]
> [task 2017-01-31T10:01:31.855715Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.856768Z] 10:01:31     INFO - #24:
> mozilla::ipc::MessagePump::Run [ipc/glue/MessagePump.cpp:97]
> [task 2017-01-31T10:01:31.858068Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.859276Z] 10:01:31     INFO - #25:
> MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:239]
> [task 2017-01-31T10:01:31.860278Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.861539Z] 10:01:31     INFO - #26: MessageLoop::Run
> [ipc/chromium/src/base/message_loop.cc:502]
> [task 2017-01-31T10:01:31.862374Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.863754Z] 10:01:31     INFO - #27:
> nsBaseAppShell::Run [widget/nsBaseAppShell.cpp:158]
> [task 2017-01-31T10:01:31.864747Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.866026Z] 10:01:31     INFO - #28:
> nsAppStartup::Run [toolkit/components/startup/nsAppStartup.cpp:284]
> [task 2017-01-31T10:01:31.867153Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.868204Z] 10:01:31     INFO - #29:
> XREMain::XRE_mainRun [toolkit/xre/nsAppRunner.cpp:4495]
> [task 2017-01-31T10:01:31.869201Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.870433Z] 10:01:31     INFO - #30:
> XREMain::XRE_main [toolkit/xre/nsAppRunner.cpp:4623]
> [task 2017-01-31T10:01:31.871563Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.872824Z] 10:01:31     INFO - #31: XRE_main
> [toolkit/xre/nsAppRunner.cpp:4714]
> [task 2017-01-31T10:01:31.873920Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.875240Z] 10:01:31     INFO - #32: do_main
> [browser/app/nsBrowserApp.cpp:261]
> [task 2017-01-31T10:01:31.876370Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.877170Z] 10:01:31     INFO - #33: main
> [browser/app/nsBrowserApp.cpp:452]
> [task 2017-01-31T10:01:31.877728Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.878533Z] 10:01:31     INFO - #34: libc.so.6 +
> 0x20830
> [task 2017-01-31T10:01:31.879036Z] 10:01:31     INFO - 
> [task 2017-01-31T10:01:31.879828Z] 10:01:31     INFO - #35: _start
> [task 2017-01-31T10:01:31.880497Z] 10:01:31     INFO - 
> 
> html:object isn't used in the codebase at the moment, skinning with CSS and
> background-image: url(...) seems to be the way to go, see
> https://dxr.mozilla.org/mozilla-central/
> search?q=path%3Abrowser%2F+svg&redirect=true
> https://dxr.mozilla.org/mozilla-central/source/browser/base/content/
> abouthome/aboutHome.css#128
> https://dxr.mozilla.org/mozilla-central/source/browser/themes/shared/search/
> search-arrow-go.svg

What if I use html:image? I cam achieve the same result. Is this ok? I thought that it was ok to use html:object. I asked Edouard and Mark's opinions in the comments above.

Is it ok to use html:image?

Comment 90

[Edouard Oger [:eoger]]

If you can achieve the same result with html:image, go ahead.

Comment 91

[Mark Hammond [:markh] [:mhammond]]

(In reply to Dorel Barbu from comment #89)
> Is it ok to use html:image?

Yes - see above:

(In reply to Dorel Barbu from comment #17)
> (In reply to Mark Hammond [:markh] from comment #16)
> > You should be able to use, eg, <html:img> in a xul file similar to
> > https://dxr.mozilla.org/mozilla-central/source/devtools/client/webide/
> > content/webide.xul#128 - does that help?
> 
> Thank you, this is helpful. But I've been unsuccessful. Apparently I can't
> use <html:img because it doesn't load the image at all.

Comment 92

[Dorel Barbu]

(In reply to Mark Hammond [:markh] from comment #91)
> (In reply to Dorel Barbu from comment #89)
> > Is it ok to use html:image?
> 
> Yes - see above:
> 
> (In reply to Dorel Barbu from comment #17)
> > (In reply to Mark Hammond [:markh] from comment #16)
> > > You should be able to use, eg, <html:img> in a xul file similar to
> > > https://dxr.mozilla.org/mozilla-central/source/devtools/client/webide/
> > > content/webide.xul#128 - does that help?
> > 
> > Thank you, this is helpful. But I've been unsuccessful. Apparently I can't
> > use <html:img because it doesn't load the image at all.

Got it, I will try the "html:image" approach. I've been very busy with my exams. Now that I'm done with it I can start working on this bug again. I'll soon post an update. Thank you! :)

Comment 93

[Dorel Barbu]

Comment on attachment 8830267 [details]
Bug 1228478 - Replaced the use of sync-illustration.png and [e-mail] with sync-illustration.svg

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/107148/diff/5-6/

Comment 94

[Dorel Barbu]

Hello everybody! I've returned with a new version of the patch. This time I've used the html:img tag. I hope it'll turn out well. If not, We'll see what we can do. Can somebody please trigger a Try run? Thank you!

Comment 95

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Try push started: https://treeherder.mozilla.org/#/jobs?repo=try&revision=00d7729a91f6

Comment 96

[Dorel Barbu]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #95)
> Try push started:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=00d7729a91f6

Thank you. Can't wait to see the results. Tell me if it's ok when it's finished, please.

Comment 97

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

It's done, and the tests passed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=00d7729a91f6 Congratulations.

Comment 98

[Dorel Barbu]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #97)
> It's done, and the tests passed:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=00d7729a91f6
> Congratulations.

Whoaaa. Awesome! I'm so satisfied!! What should I do next? Is this bug solved now?

Comment 99

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a5c0c77062c7
Replaced the use of sync-illustration.png and [e-mail] with sync-illustration.svg r=eoger,markh

Comment 100

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

If everything works as expected, you don't need to anything here. I landed the patch on the integration branch and after the tests pass, they will be added with other changes to the main firefox repository (mozilla-central). You could start fixing your next bug, of course ;)

Comment 101

[Dorel Barbu]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #100)
> If everything works as expected, you don't need to anything here. I landed
> the patch on the integration branch and after the tests pass, they will be
> added with other changes to the main firefox repository (mozilla-central).
> You could start fixing your next bug, of course ;)

Great. I'm waiting for the final verdict, then. If everything goes well here, I'll try to find a new bug.

Thank you for your support, guys! And for helping me to get acquainted with Mercurial and the Mozilla workflow.

Do any of you have some suggestions about what my next bug should be?

Comment 102

[Phil Ringnalda (:philor)]

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

Comment 103

[Mark Hammond [:markh] [:mhammond]]

(In reply to Dorel Barbu from comment #101)
> Thank you for your support, guys! And for helping me to get acquainted with
> Mercurial and the Mozilla workflow.

Thanks for your contribution - it took alot of tenacity to get this bug landed, so congratulations!

> Do any of you have some suggestions about what my next bug should be?

Bug 1166987, bug 1135943, bug 1096343, bug 994821 and bug 684121 all look like reasonable candidates. FWIW, I got these by executing a query like https://bugzilla.mozilla.org/buglist.cgi?f1=bug_mentor&list_id=13440644&o1=isnotempty&resolution=---&emailtype1=notequals&query_format=advanced&email1=%22%22&component=Firefox%20Sync%3A%20Backend&component=Sync - basically looking for Sync related bugs where a "mentor" has been specified.

I'm clearing the needinfo on all of us, but others may still chime in with some ideas.

Thanks again!

Comment 104

[Dorel Barbu]

(In reply to Mark Hammond [:markh] from comment #103)
> (In reply to Dorel Barbu from comment #101)
> > Thank you for your support, guys! And for helping me to get acquainted with
> > Mercurial and the Mozilla workflow.
> 
> Thanks for your contribution - it took alot of tenacity to get this bug
> landed, so congratulations!
> 
> > Do any of you have some suggestions about what my next bug should be?
> 
> Bug 1166987, bug 1135943, bug 1096343, bug 994821 and bug 684121 all look
> like reasonable candidates. FWIW, I got these by executing a query like
> https://bugzilla.mozilla.org/buglist.
> cgi?f1=bug_mentor&list_id=13440644&o1=isnotempty&resolution=---
> &emailtype1=notequals&query_format=advanced&email1=%22%22&component=Firefox%2
> 0Sync%3A%20Backend&component=Sync - basically looking for Sync related bugs
> where a "mentor" has been specified.
> 
> I'm clearing the needinfo on all of us, but others may still chime in with
> some ideas.
> 
> Thanks again!

Thank you, Mark! I'll look into it and decide which bug I'll try next.

Comment 105

[Matthew N. [:MattN]]

FYI this also changed the layout of the page (though I don't see anything particularly wrong with it): https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=4ec373fafebf79846cd5fde0561ac02fa0bb9647&newProject=mozilla-central&newRev=43270c88b2513a6758a0ce6669559f4a5f3c9207&filter=prefsSync

No need to reply to me about this if it's fine, it's just FYI.

Comment 106

[Mark Hammond [:markh] [:mhammond]]

Thanks Matt! Ryan, can you please confirm you are happy with this change? https://screenshots.mattn.ca/comparisons/mozilla-central/4ec373fafebf79846cd5fde0561ac02fa0bb9647/mozilla-central/43270c88b2513a6758a0ce6669559f4a5f3c9207/linux64/preferences_09_prefsSync.png seems a reasonable link to help see how it changed.

Comment 107

[Dorel Barbu]

(In reply to Mark Hammond [:markh] from comment #106)
> Thanks Matt! Ryan, can you please confirm you are happy with this change?
> https://screenshots.mattn.ca/comparisons/mozilla-central/
> 4ec373fafebf79846cd5fde0561ac02fa0bb9647/mozilla-central/
> 43270c88b2513a6758a0ce6669559f4a5f3c9207/linux64/preferences_09_prefsSync.
> png seems a reasonable link to help see how it changed.

If it's not ok, I can work something out. Thank you, guys!

Comment 108

[Ryan Feeley [:rfeeley]]

Attachment: Screenshot 2017-03-02 13.42.44.png

Looks like there is extra space that is affecting the image in other areas. Is this something you can fix or should I get a new SVG?

Comment 109

[Mark Hammond [:markh] [:mhammond]]

(In reply to Ryan Feeley [:rfeeley] from comment #108)
> Is this something you can fix or should I get a new SVG?

I believe we can fix it without any new assets. I opened bug 1344129 for this.

Thanks!

Comment 110

[Dorel Barbu]

(In reply to Mark Hammond [:markh] from comment #109)
> (In reply to Ryan Feeley [:rfeeley] from comment #108)
> > Is this something you can fix or should I get a new SVG?
> 
> I believe we can fix it without any new assets. I opened bug 1344129 for
> this.
> 
> Thanks!

I also believe so. I'll look into it.