Arshad Khan [:arshad] Assignee
	Description • 6 years ago

      No description provided.

	Brian Grinstead [:bgrins]
	Comment 2 • 6 years ago

Comment on attachment 9021141 [details] [diff] [review]
treecolimage_WIP.patch

Review of attachment 9021141 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/content/mailWidgets.js
@@ +153,5 @@
>      return this.getAttribute("label");
>    }
>  }
>  
> +class MozTreecolImage extends MozTreecolBase {

I don't expect MozTreecolBase or MozTreecol will be directly accessible on the window, but I think you'll be able to do `customElement.get("treecol")` to get a reference to the class. See https://bugzilla.mozilla.org/show_bug.cgi?id=1499423#c25

	Arshad Khan [:arshad] Assignee
	Comment 3 • 6 years ago

(In reply to Brian Grinstead [:bgrins] from comment #2)
> Comment on attachment 9021141 [details] [diff] [review]
> treecolimage_WIP.patch
> 
> Review of attachment 9021141 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mail/base/content/mailWidgets.js
> @@ +153,5 @@
> >      return this.getAttribute("label");
> >    }
> >  }
> >  
> > +class MozTreecolImage extends MozTreecolBase {
> 
> I don't expect MozTreecolBase or MozTreecol will be directly accessible on
> the window, but I think you'll be able to do `customElement.get("treecol")`
> to get a reference to the class. See
> https://bugzilla.mozilla.org/show_bug.cgi?id=1499423#c25

thanks for letting me know again.

	Jorg K (CEST = GMT+2)
	Comment 4 • 6 years ago

This has become urgent since we have 217 test failures over in bug 1504076.

	Richard Marti (:Paenglab)
	Comment 5 • 6 years ago

Comment on attachment 9021141 [details] [diff] [review]
treecolimage_WIP.patch

I tried this patch.

First it doesn't apply on tip.

The treecol-image have no <image class="treecol-icon"> as child and so no icon is shown.

	Arshad Khan [:arshad] Assignee
	Comment 6 • 6 years ago

(In reply to Richard Marti (:Paenglab) from comment #5)
> Comment on attachment 9021141 [details] [diff] [review]
> treecolimage_WIP.patch
> 
> I tried this patch.
> 
> First it doesn't apply on tip.
> 
> The treecol-image have no <image class="treecol-icon"> as child and so no
> icon is shown.

just about to post a new patch.. my build is goin on.

	Arshad Khan [:arshad] Assignee
	Comment 8 • 6 years ago

the 30 errors in console are not due to this patch.. I ll look into that in a new bug if required.

	Magnus Melin [:mkmelin]
	Comment 9 • 6 years ago

Comment on attachment 9022096 [details] [diff] [review]
treecolimage.patch

Review of attachment 9022096 [details] [diff] [review]:
-----------------------------------------------------------------

I'll make these changes and land it. r=mkmelin

::: mail/base/content/SearchDialog.xul
@@ +128,5 @@
>                context="mailContext"
>                onkeydown="ThreadPaneKeyDown(event);"
>                onselect="ThreadPaneSelectionChanged();">
>            <treecols id="threadCols" pickertooltiptext="&columnChooser2.tooltip;">
> +            <treecol is="treecol-image" id="threadCol" persist="hidden ordinal" fixed="true" cycler="true"

would prefer id first for these

::: mail/base/content/mailWidgets.js
@@ +272,5 @@
> +    }
> +
> +    const src = this.getAttribute("src");
> +
> +    if (src != null) {

and remove attr for when it's null

	Arshad Khan [:arshad] Assignee
	Comment 10 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=22419d99937c9883c3bcfdbcd21324b4d438556c

	Richard Marti (:Paenglab)
	Comment 11 • 6 years ago

Comment on attachment 9022096 [details] [diff] [review]
treecolimage.patch

Tested on Linux and Windows. The treecol-icons are where they should be.

	Pulsebot
	Comment 12 • 6 years ago

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/8b5b4aa26193
Adding treecol-image custom element; r=mkmelin