1502988 - Reimplementing treecol-image after treecol is converted into custom element.

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Comment 1

[Arshad Khan [:arshad]]

Attachment: treecolimage_WIP.patch

Comment 2

[Brian Grinstead [:bgrins]]

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

Comment 3

[Arshad Khan [:arshad]]

(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.

Comment 4

[Jorg K (CEST = GMT+2)]

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

Comment 5

[Richard Marti (:Paenglab)]

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.

Comment 6

[Arshad Khan [:arshad]]

(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.

Comment 7

[Arshad Khan [:arshad]]

Attachment: treecolimage.patch

Comment 8

[Arshad Khan [:arshad]]

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

Comment 9

[Magnus Melin [:mkmelin]]

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

Comment 10

[Arshad Khan [:arshad]]

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

Comment 11

[Richard Marti (:Paenglab)]

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

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8b5b4aa26193
Adding treecol-image custom element; r=mkmelin