bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

Promisify _removeLegacySites from content/newtab/updater.js

RESOLVED FIXED in Firefox 60

Status

()

Firefox
New Tab Page
RESOLVED FIXED
4 years ago
4 months ago

People

(Reporter: erikvold, Assigned: erikvold)

Tracking

Trunk
Firefox 60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(1 attachment)

Comment hidden (empty)
Blocks: 1075217
Assignee: nobody → evold
Comment on attachment 8511670 [details] [diff] [review]
1089368.diff

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

Thanks! r=me with the two callback args removed.

::: browser/base/content/newtab/updater.js
@@ +139,5 @@
>            node.parentNode.removeChild(node);
>            resolve();
>          });
> +      });
> +    }).then(aCallback).catch(console.exception);

There's no reason to still support the callback, right? Catching errors should probably be moved to the caller as well.

@@ +152,4 @@
>      let {cells, sites} = gGrid;
>  
>      // Find empty cells and fill them.
> +    return Promise.all(sites.map((aSite, aIndex) => {

We can remove the callback argument here for _fillEmptyCells() as well.
Attachment #8511670 - Flags: review?(ttaubert) → review+
(In reply to Tim Taubert [:ttaubert] from comment #2)
> Comment on attachment 8511670 [details] [diff] [review]
> 1089368.diff
> 
> Review of attachment 8511670 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks! r=me with the two callback args removed.
> 
> ::: browser/base/content/newtab/updater.js
> @@ +139,5 @@
> >            node.parentNode.removeChild(node);
> >            resolve();
> >          });
> > +      });
> > +    }).then(aCallback).catch(console.exception);
> 
> There's no reason to still support the callback, right? Catching errors
> should probably be moved to the caller as well.
> 
> @@ +152,4 @@
> >      let {cells, sites} = gGrid;
> >  
> >      // Find empty cells and fill them.
> > +    return Promise.all(sites.map((aSite, aIndex) => {
> 
> We can remove the callback argument here for _fillEmptyCells() as well.

sounds good!
Code was removed in bug 1433133 https://hg.mozilla.org/mozilla-central/rev/fc80dd3e4fae
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox60: --- → fixed
Depends on: 1433133
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
You need to log in before you can comment on or make changes to this bug.