Port Bug 1374282 [Switch to async/await from Task.jsm/yield] to SeaMonkey

RESOLVED FIXED in seamonkey2.55

Status

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: bugZ, Assigned: frg)

Tracking

SeaMonkey 2.54 Branch
seamonkey2.55
Dependency tree / graph

SeaMonkey Tracking Flags

(seamonkey2.53 fixed, seamonkey2.54 fixed, seamonkey2.55 fixed)

Details

Attachments

(3 attachments, 4 obsolete attachments)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0 SeaMonkey/2.54a1
Build ID: 20170815055359

Steps to reproduce:

Starting with the 2017-08-16 build (both Win32 and Win64), Data Manager appears broken.
1. Start the browser
2. In the location bar, enter about:data


Actual results:

Data Manager is blank. The error console shows 3 errors (will attach screenshot):
ReferenceError:gDataman is not defined
ReferenceError:gTabs is not defined
Syntax Error: yield expression is only valid in generators

The 2017-08-26 build also shows:
FullZoom is undefined


Expected results:

Expected Data Manager to show the domains and what is stored for each.
Assignee: nobody → frgrahl
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Summary: Data Manager looks empty - gDataman is not defined → Port Bug 1374282 [Switch to async/await from Task.jsm/yield] to SeaMonkey
Posted patch 1394101-yield-WIP.patch (obsolete) — Splinter Review
The startup error is gone but still need to do the Data Manager patch.
Posted patch 1394101-yield-WIP.patch (obsolete) — Splinter Review
Attachment #8901498 - Attachment is obsolete: true
Posted patch 1394101-yield.patch (obsolete) — Splinter Review
Ran the script from bug 1374282 against suite. This only left the Data Manager and these two were an easy fix. Looks like just the declarations to make it generator funtions were missing. I wonder if this was an error before or I miss something? 

Tested and working as far as I see it.
Attachment #8901517 - Attachment is obsolete: true
Attachment #8901583 - Flags: review?(iann_bugzilla)
Posted patch 1394101-yield.patch (obsolete) — Splinter Review
Comment adjusted. Bug number was missing. Patch not changed.
Attachment #8901583 - Attachment is obsolete: true
Attachment #8901583 - Flags: review?(iann_bugzilla)
Attachment #8902003 - Flags: review?(iann_bugzilla)
Is this bug the likely reason form field "suggestions" also stopped working?
Probably but might be Bug 1392929. They are working in my local build with both patches applied.
Comment on attachment 8902003 [details] [diff] [review]
1394101-yield.patch

>+++ b/suite/common/dataman/dataman.js
>-    function loader() {
>+    function* loader() {

>-    function loader() {
>+
>+    function* loader() {

Are these changes to do with Task/yield as I cannot find similar in the reference patch.

>+++ b/suite/common/places/tests/autocomplete/test_enabled.js
>@@ -27,17 +27,17 @@ var gTests = [
>   ["3: resume normal search",
>    "url", [0], () => setSearch(1)],
> ];
> 
> function setSearch(aSearch) {
>   prefs.setBoolPref("browser.urlbar.autocomplete.enabled", !!aSearch);
> }
> 
>-add_task(function* test_sync_enabled() {
>+add_task(async function test_sync_enabled() {
>   // Initialize autocomplete component.
>   Cc["@mozilla.org/autocomplete/search;1?name=history"]
>     .getService(Ci.mozIPlacesAutoComplete);
> 
>   let types = [ "history", "bookmark", "openpage" ];
> 
>   // Test the service keeps browser.urlbar.autocomplete.enabled synchronized
>   // with browser.urlbar.suggest prefs.
Is this change correct as there is no yield in the function?

r=me with those points answered/addressed
Attachment #8902003 - Flags: review?(iann_bugzilla) → review+
> Are these changes to do with Task/yield as I cannot find similar in the reference patch.

I think the code was incorrect but there were many changes in this area and I didn't find the bug where syntax checking was tighted up. I am splitting up the patch into two parts and change the comment. If needed we can take this on to other trees or back it out and put in a new comment.

> Is this change correct as there is no yield in the function?

The corresponding file is
mozilla/toolkit/components/places/tests/unifiedcomplete/test_enabled.js

It was changed in Bug 1353542
https://bug1353542.bmoattachments.org/attachment.cgi?id=8865250

I believe the test is and was broken and needs to be adapted. Previously maybe called as a generator but I am not seeing it now.

I will add Bug 1353542 to the comment. This seem to be the main bug for the changes.
Split patch Part 1 addressing reviewer comments. r+ from IanN carried forward.

Drat! Thought I had time till Monday but Merges just started. Well I wanted it in former Beta which is now release anyway :)

[Approval Request Comment]
Regression caused by (bug #): 1394101
User impact if declined: 2.54 broken
Testing completed (on m-c, etc.): c-c c-b
Risk to taking this patch (and alternatives if risky): low risk. Tested.
String changes made by this patch: none.
Attachment #8902003 - Attachment is obsolete: true
Attachment #8911203 - Flags: review+
Attachment #8911203 - Flags: approval-comm-release?
Attachment #8911203 - Flags: approval-comm-beta?
Split patch Part 2 for Data Manager. Addressing reviewer comments. r+ from IanN carried forward.

[Approval Request Comment]
Regression caused by (bug #): 1394101
User impact if declined: 2.54 Data Manager broken
Testing completed (on m-c, etc.): c-c c-b
Risk to taking this patch (and alternatives if risky): low risk. Tested.
String changes made by this patch: none.
Attachment #8911205 - Flags: review+
Attachment #8911205 - Flags: approval-comm-release?
Attachment #8911205 - Flags: approval-comm-beta?
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/56a768de610b
Part 1 Port Bug 1374282 [Switch to async/await from Task.jsm/yield] to SeaMonkey. r=IanN
https://hg.mozilla.org/comm-central/rev/264f1743605c
Part 2 Add missing generator function declare in Data Manager. r=IanN
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.55
Comment on attachment 8911203 [details] [diff] [review]
1394101-part1-yield.patch

a=me
Attachment #8911203 - Flags: approval-comm-release?
Attachment #8911203 - Flags: approval-comm-release+
Attachment #8911203 - Flags: approval-comm-beta?
Attachment #8911203 - Flags: approval-comm-beta+
Comment on attachment 8911205 [details] [diff] [review]
1394101-part2-yield.patch

a=me
Attachment #8911205 - Flags: approval-comm-release?
Attachment #8911205 - Flags: approval-comm-release+
Attachment #8911205 - Flags: approval-comm-beta?
Attachment #8911205 - Flags: approval-comm-beta+
No longer blocks: 2.56BulkMalfunctions
You need to log in before you can comment on or make changes to this bug.