Closed Bug 1517523 Opened 6 years ago Closed 6 years ago

[de-xbl] Migrate results-message to custom element.

Categories

(Thunderbird :: Mail Window Front End, task)

Product:
Thunderbird
Component:
Mail Window Front End
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 67.0

People

(Reporter: arshad, Assigned: arshad)

References

Details

Attachments

(1 file, 6 obsolete files)

results-message.patch
(deleted), patch
arshad
: review+
Details | Diff | Splinter Review 
Binding Link - https://searchfox.org/comm-central/source/mail/base/content/glodaFacetBindings.xml#1166
Attached patch results-message.patch (obsolete) (deleted) — — Splinter Review 
Hey Magnus,

This patch appears to successfully render results but somehow it messes up the facet visualization. I get `this.mm is null` where this.mm is 
 asssigned docshell.messageManager. Could you please take a look at this and let me know which part of the patch is messing up things
?
Assignee: nobody → arshdkhn1
Flags: needinfo?(mkmelin+mozilla)
Blocks: tb-war-on-xbl
Depends on: 1521112

The this.mm problem seems to be because of some bug in objectTreeAsString
If I temporarily remove that, I get the real problem

rules is undefined glodaFacetVis.js:176
render chrome://messenger/content/glodaFacetVis.js:176
build chrome://messenger/content/glodaFacetVis.js:28
buildFunc chrome://messenger/content/glodaFacet.js:53
facetingCompleted chrome://messenger/content/glodaFacetView.js:585
_drive resource:///modules/gloda/facet.js:107
FacetDriver_go resource:///modules/gloda/facet.js:80
build chrome://messenger/content/glodaFacetView.js:530
initialBuild chrome://messenger/content/glodaFacetView.js:493
onQueryCompleted chrome://messenger/content/glodaFacetView.js:963
GlodaMsgSearcher_onQueryCompleted resource:///modules/gloda/msg_search.js:339
gloda_coll_onQueryCompleted resource:///modules/gloda/collection.js:626
onQueryCompleted resource:///modules/gloda/datastore.js:149
gloda_coll_onQueryCompleted resource:///modules/gloda/collection.js:626
onQueryCompleted resource:///modules/gloda/datastore.js:149
gloda_coll_onQueryCompleted resource:///modules/gloda/collection.js:626
onQueryCompleted resource:///modules/gloda/datastore.js:149
gloda_coll_onQueryCompleted resource:///modules/gloda/collection.js:626
onQueryCompleted resource:///modules/gloda/datastore.js:149
gloda_ds_qfq_handleCompletion resource:///modules/gloda/datastore.js:390

Flags: needinfo?(mkmelin+mozilla)
Attached patch results-message.patch (obsolete) (deleted) — — Splinter Review

Unbitrotted and with the temporary uncommenting. You should be able to get on with this bug now

Oh, the unbitrotted patch also need to remove glodaFacetBindings.xml

Attached patch results-message.patch (obsolete) (deleted) — — Splinter Review

Now with the lodaFacetBindings.xml file removed

Attachment #9034211 - Attachment is obsolete: true
Attachment #9043874 - Attachment is obsolete: true
Attached patch results-message.patch (obsolete) (deleted) — — Splinter Review
Attachment #9043999 - Attachment is obsolete: true
Attachment #9045674 - Flags: review?(mkmelin+mozilla)
Attached patch results-message.patch (obsolete) (deleted) — — Splinter Review
Attachment #9045674 - Attachment is obsolete: true
Attachment #9045674 - Flags: review?(mkmelin+mozilla)
Attachment #9045675 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9045675 [details] [diff] [review]
results-message.patch

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

LGTM, r=mkmelin with some nits fixed

Also run it past try before setting checkin-needed

::: mail/base/content/glodaFacet.js
@@ +64,5 @@
>    }
>  }
>  
> +/**
> + * MozResultsMessage shows up the search results for the string entered in gloda-searchbox.

remove the "up"

@@ +223,2 @@
>  customElements.define("facet-date", MozFacetDate);
> +customElements.define("facet-results-message", MozResultsMessage);

Please make the class name MozFacetResultsMessage
Attachment #9045675 - Flags: review?(mkmelin+mozilla) → review+
Attached patch results-message.patch (obsolete) (deleted) — — Splinter Review

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

Attachment #9045675 - Attachment is obsolete: true
Attachment #9045901 - Flags: review+
Attached patch results-message.patch (deleted) — — Splinter Review
Attachment #9045901 - Attachment is obsolete: true
Attachment #9045903 - Flags: review+

Removed empty gldaFacetBindings.css file.

(In reply to Arshad Khan [:arshad] from comment #9)

Created attachment 9045901 [details] [diff] [review]
results-message.patch

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

I need to rebase the patch, but I am not sure whether trunk is failing or not.

You just need to rebase, trunk is fine atm.

Keywords: checkin-needed

The tests that are failing are because of the tree stuff.

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/f87c5afa28b8
Migrate results-message binding to custom element. r=mkmelin

Status: NEW → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 67.0
Type: enhancement → task
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: