Fix Bug 1433230 - Add recent downloads to Highlights #4076

sarracini · 2018-04-03T19:29:18Z

Adds a single, most recent download to highlights. Right now just appends it to the end of all highlights, so the easiest way to test this is on a new profile, with little history and maybe just a bookmark.
Some notes:

Clicking on the card opens it up in the application that the OS can open it in
Show in Finder string is determined based on platform, and is only shown if there is a path to show
Go to Download page option in the context menu is disabled if the download doesn't have a referrer.
Clicking on the magnifying glass should do the same as Show in Finder
We refresh highlights when a new download gets added, or when a download gets deleted, so that it can show up immediately (like bookmarks)
We only care about downloads that are newer than 36 hours
Highlights feed needs to make an instance of DownloadsManager, but DownloadsManager needs know about actions that are dispatched to it. This means it would also need to be instantiated in ActivityStream.jsm so it can be hooked up to the store. Since we don't want 2 instances of DownloadsManager, HighlightsFeed relays actions to DownloadsManager.

Designs: https://mozilla.invisionapp.com/share/J6GEU4EHTYC#/screens

Here's a picture!

sarracini · 2018-04-05T20:22:13Z

system-addon/lib/ActivityStreamMessageChannel.jsm

@@ -241,7 +241,7 @@ this.ActivityStreamMessageChannel = class ActivityStreamMessageChannel {
      browser.renderLayers = true;
    }

-    this.onActionFromContent({type: at.NEW_TAB_LOAD}, msg.target.portID);
+    this.onActionFromContent({type: at.NEW_TAB_LOAD, _target: msg.target}, msg.target.portID);


DownloadsCommon.getData needs a browser, so we just tack on the browser here so that I can access it in DownloadsManager.init()

This set of changes relating to _target shouldn't be needed anymore?

sarracini · 2018-04-05T20:23:45Z

system-addon/lib/DownloadsManager.jsm

+    this._viewableDownloadItems = new Map();
+  }
+
+  addStore(store) {


I'm not too crazy about this solution here... any better ideas are welcome @Mardak / @k88hudson

Should this just be a Feed, which would automatically get a store? I suppose the sub option for highlights would just directly toggle the feed on/off… ??

sarracini · 2018-04-05T20:28:01Z

system-addon/test/unit/lib/DownloadsManager.test.js

+  describe("#onAction", () => {
+    it("should add a DownloadsCommon view on NEW_TAB_LOAD", () => {
+      let stub = global.DownloadsCommon.getData().addView;
+      stub.reset();


I think stub.reset() may be removed from their API soon...... ¯_(ツ)_/¯

Mardak

Playing around with the PR felt somewhat odd -- partially because the hovered url in the "status bar" is not what happens on click and also just the behavior of launching applications. We probably want some security feedback on adding this feature…

Also, the new tab -> init behavior and your addStore question seems to favor a Feed as opposed to the current behavior that seems to accidentally work (??) when addView of the same thing multiple times.

I think we should chat more about the approach and reevaluate before touching too much more.

Mardak · 2018-04-09T22:23:20Z

locales/en-US/strings.properties

+# link that belongs to this downloaded item"
+menu_action_copy_download_link=Copy Download link
+menu_action_go_to_download_page=Go to Download page
+menu_action_remove_download=Remove from history


These strings don't seem to match the casing of our other menu strings. In particular it looks like words are generally uppercase except prepositions?

Yeah I know, I've been back and forth with design and they told me to be consistent with our context menus in activity stream

It would seem that being "consistent with our context menus" would mean these strings should be "Copy Download Link" "Go to Download Page" "Remove from History" etc… ?

Mardak · 2018-04-09T22:23:20Z

system-addon/content-src/lib/link-menu-options.js

+  ShowFile: (site, index, eventSource, isEnabled, siteInfo, platform) => ({
+    id: _GetPlatformString(platform),
+    icon: "search",
+    action: ac.AlsoToMain({


Do these actions want to be Also or just Only? I don't think content does anything with these?

Ah good call, these should be Only

Mardak · 2018-04-09T22:23:20Z

system-addon/lib/DownloadsManager.jsm

+    if (this._store.getState().Prefs.values["section.highlights.includeDownloads"]) {
+      this._browser = action._target.browser;
+      this._downloadData = DownloadsCommon.getData(this._browser.ownerGlobal, true, false, true);
+      this._downloadData.addView(this);


This doesn't seem quite right of attempting to add a per-tab view vs a single activity stream view / feed. It seems like this actually doesn't result in multiple views as addView(this) happens to try to add the same object on a second init? Although then does this have problems on tab close / uninit if called multiple times?

Mardak · 2018-04-09T22:23:20Z

system-addon/lib/DownloadsManager.jsm

+    this._viewableDownloadItems = new Map();
+  }
+
+  addStore(store) {


Should this just be a Feed, which would automatically get a store? I suppose the sub option for highlights would just directly toggle the feed on/off… ??

Mardak · 2018-04-09T22:23:20Z

system-addon/lib/DownloadsManager.jsm

+        break;
+      case at.OPEN_LINK: {
+        if (action.data.type === "download") {
+          this.openFile(action.data.url);


I suppose this is by design, but in testing, something feels a bit wrong to launch files / applications by clicking a link in content space… Somewhat related of how to prevent malicious launching of other files? I suppose this happens to require a file be downloaded.. Although this "type" could be faked…

Mardak · 2018-04-09T22:23:20Z

system-addon/lib/DownloadsManager.jsm

+    const downloadedItem = element.download;
+    return {
+      hostname: new URL(downloadedItem.source.url).hostname,
+      url: downloadedItem.source.url,


This causes the status / hovered link url (at the bottom corner of the window) to show up as the download source, but clicking the link / card takes you elsewhere.

Did design have any comments on what should show in the status bar on hover?

k88hudson · 2018-04-10T17:58:11Z

system-addon/lib/HighlightsFeed.jsm

@@ -260,6 +274,18 @@ this.HighlightsFeed = class HighlightsFeed {
      case at.UNINIT:
        this.uninit();
        break;
+
+      // Relay the downloads actions to DownloadsManager
+      case at.NEW_TAB_LOAD:


You don't need to do this, you can just call this.downloadsManager.onAction(action); on every action

sarracini · 2018-04-10T19:05:34Z

Chatted with Bryan, and the following changes are to be made:

remove the little hover search glass thing in the corner
clicking on the card opens it in finder
Show in Finder in context menu then becomes Open File
add an icon to the card to make it clear that it's a download

Mardak

Main question of how spread out all this download type checking should be.. in particular for Card component. But also any place with a type check, I wonder if it should be checking for behaviors or other properties instead.

There's also a variety of cleanups and fixes.

Mardak · 2018-04-12T09:24:28Z

locales/en-US/strings.properties

+# link that belongs to this downloaded item"
+menu_action_copy_download_link=Copy Download link
+menu_action_go_to_download_page=Go to Download page
+menu_action_remove_download=Remove from history


It would seem that being "consistent with our context menus" would mean these strings should be "Copy Download Link" "Go to Download Page" "Remove from History" etc… ?

Mardak · 2018-04-12T09:24:28Z

system-addon/content-src/components/Card/Card.jsx

+    // string for "Show in Finder". Else, show the regular hostname
+    if (this.props.link.type === "download") {
+      if (this.state.isHover) {
+        return <FormattedMessage id={GetPlatformString(this.props.platform)} defaultMessage={GetPlatformString("default")} />;


This handling of hover with react seems a bit excessive? Could just always render both primary and alternate hostname texts and show / hide with css. Or maybe that's too complicated too ;)

<div class="card-host-name alternate">Show in Finder</div> <div class="card-host-name">hostname</div>

.card-outer .card-host-name.alternate, .card-outer:hover .alternate ~ .card-host-name { display: none; } .card-outer:hover .card-host-name.alternate { display: block; }

Hmm, I suppose this is actually probably more reliable since we don't have to deal with react mouse over events blah blah blah

Mardak · 2018-04-12T09:24:28Z

system-addon/content-src/components/Card/_Card.scss

+    margin-top: 2px;
+
+    &.icon-download-folder {
+      background-size: $small-download-folder-icon-size;


This can just be 100% and not need to explicitly set it for wider.

Mardak · 2018-04-12T09:24:28Z

system-addon/content-src/components/Card/_Card.scss

@@ -129,6 +129,18 @@
    text-transform: uppercase;
  }

+  .card-download-icon {
+    float: right;


This should be inline-end instead of right.

Mardak · 2018-04-12T09:24:28Z

system-addon/content-src/components/LinkMenu/LinkMenu.jsx


    // Handle special case of default site
    const propOptions = !site.isDefault ? props.options : DEFAULT_SITE_MENU_OPTIONS;

-    const options = propOptions.map(o => LinkMenuOptions[o](site, index, source, isPrivateBrowsingEnabled, siteInfo)).map(option => {
+    const options = propOptions.map(o => LinkMenuOptions[o](site, index, source, isPrivateBrowsingEnabled, siteInfo, platform)).map(option => {


Kinda looks like we just want to pass in props as the argument for each LinkMenuOptions so each can destructure its one parameter to pick out what is actually desired… eh

Mardak · 2018-04-12T09:24:28Z

system-addon/lib/DownloadsManager.jsm

+ChromeUtils.defineModuleGetter(this, "DownloadsCommon",
+  "resource:///modules/DownloadsCommon.jsm");
+
+const THIRTY_SIX_HOURS = 36 * 60 * 60 * 1000;


Let's name this something more describing "recent download threshold" than just a text of the value.

Mardak · 2018-04-12T09:24:28Z

system-addon/lib/DownloadsManager.jsm

+        this.showFile(action.data.url);
+        break;
+      case at.OPEN_DOWNLOAD_FILE:
+        this.openFile(action.data.url);


Eh.. these 5 actions could just just do something like

case OPEN: downloadsCmd = "open"; break; … if (downloadsCmd) { this.findDownloadedItem(action.data.url)[`downloadsCmd_${downloadsCmd}`](); }

Too clever? ;)

Yeah I thought about this too. I don't mind being too clever ;)

Mardak · 2018-04-12T09:24:28Z

system-addon/content-src/components/Card/Card.jsx

        <div className="card">
          {hasImage && <div className="card-preview-image-outer">
            <div className={`card-preview-image${this.state.imageLoaded ? " loaded" : ""}`} style={imageStyle} />
          </div>}
          <div className={`card-details${hasImage ? "" : " no-image"}`}>
-            {link.hostname && <div className="card-host-name">{link.hostname}</div>}
+            {link.type === "download" && <div className="card-download-icon icon icon-download-folder" />}
+            {link.hostname && <div className="card-host-name">{this.computeHostname()}</div>}


The Card component has been relatively generic allowing custom behavior via additional optional properties as opposed to type checking.

Alternatively, it looks like the Download Card looks and behaves a bit differently from existing Cards, so maybe it should just be its own Component?

@k88hudson any thoughts on if we should care about how generic Card is? It also would potentially affect web extensions that happen to have use a "download" type.. and more generally of special type behaviors.

Mardak · 2018-04-12T09:24:28Z

system-addon/lib/PlacesFeed.jsm

@@ -313,7 +313,9 @@ class PlacesFeed {
        this.saveToPocket(action.data.site, action._target.browser);
        break;
      case at.OPEN_LINK: {
-        this.openLink(action);
+        if (action.data.type !== "download") {


This check / change shouldn't be needed anymore.

Mardak · 2018-04-12T09:24:28Z

system-addon/lib/DownloadsManager.jsm

+    const downloadedItem = element.download;
+    return {
+      hostname: new URL(downloadedItem.source.url).hostname,
+      url: downloadedItem.source.url,


Did design have any comments on what should show in the status bar on hover?

sarracini · 2018-04-12T13:40:01Z

A note for myself: Make sure the private browsing downloads don't show up

sarracini · 2018-04-12T14:00:38Z

Also @Mardak design said they didn't care about how the status bar shows the url

sarracini · 2018-04-16T19:46:25Z

Recent discoveries blocking this bug:

A bug in Places means that we receive onDownloadAdded notifications for every page that we visit. This will most likely show up in profiles, so until that's been optimized for we should hold off on that
As per the second part of this comment, we should not be initializing the Download system (i.e calling DownloadsCommon.getData on startup, since it does a lot of work and will most likely be bad news. A possible solution would be to dispatch a "downloaded added" notification in History.cpp -> AddDownload and listen for that in DownloadsManager.jsm and only initialize once we've received it

Mardak

Looks like some things aren't quite working, but we can file some followup bugs for them. Also I noticed downloads window shows the time. Was that intentionally not included by design?

Mardak · 2018-04-18T21:49:20Z

system-addon/content-src/components/Card/Card.jsx

+      }));
+    } else {
+      const {altKey, button, ctrlKey, metaKey, shiftKey} = event;
+      this.props.dispatch(ac.AlsoToMain({


nit: we can fix this up too to Only

Mardak · 2018-04-18T21:49:20Z

system-addon/content-src/components/Card/Card.jsx

@@ -146,6 +154,8 @@ export class Card extends React.PureComponent {
            <div className={`card-preview-image${this.state.imageLoaded ? " loaded" : ""}`} style={imageStyle} />
          </div>}
          <div className={`card-details${hasImage ? "" : " no-image"}`}>
+            {link.type === "download" && <div className="card-download-icon icon icon-download-folder" />}
+            {link.type === "download" && <div className="card-host-name alternate"><FormattedMessage id={GetPlatformString(this.props.platform)} defaultMessage={GetPlatformString("default")} /></div>}


Do we need a defaultMessage? It would only be used if the provided id is missing? Our string packaging will fall back to english or something if a locale didn't translate a string. I suppose it could be possible that we remove a string and it'll switch to the default maybe.. but that should probably be more visible anyway?

Yup we can remove this, I think it was laying around from before when I didn't have it always showing an alternate host 👍

Mardak · 2018-04-18T21:49:20Z

locales/en-US/strings.properties

@@ -22,6 +22,7 @@ type_label_visited=Visited
 type_label_bookmarked=Bookmarked
 type_label_recommended=Trending
 type_label_pocket=Saved to Pocket
+type_label_download=Downloaded


nit: we have type_label_bookmarked so let's do "downloaded" in the id.

Mardak · 2018-04-18T21:49:20Z

system-addon/content-src/components/Card/_Card.scss

@@ -49,6 +49,9 @@
    .card-title {
      color: var(--newtab-link-primary-color);
    }
+
+    .alternate ~ .card-host-name { display: none; }
+    .card-host-name.alternate { display: block; }


nit: bracing new lines

Mardak · 2018-04-18T21:49:20Z

system-addon/lib/DownloadsManager.jsm

+        downloadsCmd = "copyLocation";
+        break;
+      case at.GO_TO_DOWNLOAD_PAGE:
+        downloadsCmd = "openReferrer";


This seems to trigger an exception this.element.ownerGlobal.openURL is not a function DownloadsViewUI.jsm:453
https://searchfox.org/mozilla-central/rev/59a9a86553e9bfd9277202748ff791fd9bc0713b/browser/components/downloads/DownloadsViewUI.jsm#452-453

Mardak · 2018-04-18T21:49:20Z

system-addon/lib/DownloadsManager.jsm

+    const downloadedItem = elem.download;
+    if (!this._viewableDownloadItems.has(downloadedItem.source.url)) {
+      this._viewableDownloadItems.set(downloadedItem.source.url, elem);
+      this._store.dispatch({type: at.DOWNLOAD_CHANGED});


Looks like this is called when it first starts a download or an existing download. We probably want handle downloads that were just started and then completed to get the correct file size, otherwise:

Dismissing some page forces a refresh and it correctly shows:

Mardak · 2018-04-18T21:49:20Z

system-addon/lib/DownloadsManager.jsm

+        downloadsCmd = "openReferrer";
+        break;
+      case at.REMOVE_DOWNLOAD_FILE:
+        downloadsCmd = "delete";


Looks like it's possible for this to throw if the file was deleted.. although not sure why it doesn't show up as disabled?

[Exception... "Component returned failure code: 0x80520006 (NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) [nsIFile.isExecutable]" nsresult: "0x80520006 (NS_ERROR_FILE_TARGET_DOES_NOT_EXIST)" location: "JS frame :: resource:///modules/DownloadsCommon.jsm :: openDownloadedFile :: line 423" data: no]

Mardak · 2018-04-18T21:49:20Z

system-addon/lib/DownloadsManager.jsm

+    if (downloadsCmd) {
+      let elem = this._viewableDownloadItems.get(action.data.url);
+      if (elem) {
+        elem[`downloadsCmd_${downloadsCmd}`]();


I've learned that dynamically building keys for strings, methods, etc. is nice looking but makes it hard to find, e.g., from searchfox. "Are there any more callers of downloadsCmd_delete? No? Baleeted!" Sorry for giving the example with string building in the previous comment. :(

😂 no worries!

sarracini · 2018-04-19T17:49:24Z

Latest changes doesn't fix the whole deleted from system still showing up in highlights situation, but addresses all the other changes

Mardak · 2018-04-19T18:40:07Z

system-addon/lib/DownloadsManager.jsm

+    const elem = new DownloadElement(download, this._browser);
+    const downloadedItem = elem.download;
+    if (!this._viewableDownloadItems.has(downloadedItem.source.url)) {
+      if (downloadedItem.succeeded && downloadedItem.target.exists) {


Between these two if blocks, add await downloadedItem.refresh(); and that should get us the correct exists value.

Probably add a comment too about why

Mardak · 2018-04-19T18:42:03Z

system-addon/lib/DownloadsManager.jsm

+  get downloads() {
+    let results = [];
+    for (const url of this._viewableDownloadItems.keys()) {
+      const elem = this._viewableDownloadItems.get(url);


nit: for (const elem of .values()) as we don't use the url

Mardak

Let's fix the download exists stuff and some more cleaning, and this should be good!

Mardak · 2018-04-19T18:44:14Z

system-addon/lib/DownloadsManager.jsm

+  copyLocation(url) {
+    let elem = this._viewableDownloadItems.get(url);
+    if (elem && elem.download.target.exists) {
+      elem.downloadsCmd_copyLocation();


As noted on irc, we can do the method strings just not string concatenation. And we shouldn't need a separate exists check. If the user does delete the file while the card is already showing... oh well?

Mardak · 2018-04-19T20:40:45Z

system-addon/lib/DownloadsManager.jsm

+        // If we only care about downloads where the file still exists, refresh them
+        // to ensure the 'exists' attribute is up to date
+        if (onlyExists) {
+          await elem.download.refresh();


As discussed on irc, I'm assuming refresh, which results in file IO, is significantly slower than anything else we do in this method whether that's iterating or sorting, so we want to minimize the number of times we call it. This can be done by only checking the first few most recent downloads in order, so if the first one exists, we're done.

Mardak · 2018-04-19T20:43:39Z

system-addon/lib/DownloadsManager.jsm

-        results.push(formattedDownloadForHighlights);
+    for (const elem of this._viewableDownloadItems.values()) {
+      // Only get downloads within the time threshold specified
+      if (elem && (elem.downloadedSince < threshold)) {


Also just documenting from irc. elem shouldn't ever be falsey now. And this comparison expands to Date.now() - endTime < threshold and gets computed for every download. We can shuffle things around to be Date.now() - threshold < endTime which makes the math loop invariant(-ish), so compute a timeThreshold = Date.now() - threshold once before the loop and check endTime > timeThreshold

Mardak · 2018-04-19T21:44:24Z

system-addon/lib/DownloadsManager.jsm

+      if (onlyExists) {
+        // Refresh download to ensure the 'exists' attribute is up to date
+        await elem.download.refresh();
+        if (elem.download.target.exists && elem.download.succeeded) {


you can if (!exists) continue; and share the formatDownload / else stuff. Both paths want only succeeded downloads too?

maybe pass succeeded as a parameter too?

I suppose an optional succeeded parameter would match and doesn't need to add an extra branch for test coverage

Mardak

Should be good with tests passing!

Mardak · 2018-04-19T22:24:39Z

system-addon/lib/DownloadsManager.jsm

+      if (onlyExists) {
+        // Refresh download to ensure the 'exists' attribute is up to date
+        await elem.download.refresh();
+        if (elem.download.target.exists && elem.download.succeeded) {


I suppose an optional succeeded parameter would match and doesn't need to add an extra branch for test coverage

Mardak · 2018-04-19T22:24:39Z

system-addon/lib/DownloadsManager.jsm

+  }
+
+  downloadsCmd_openReferrer() {
+    this.element.openNewTabWith(this.download.source.referrer);


Looks like this opens in a background tab. Maybe just pass in true as the second argument for shifted?

Mardak · 2018-04-19T22:24:39Z

system-addon/lib/DownloadsManager.jsm

+    if (!this._viewableDownloadItems.has(downloadedItem.source.url)) {
+      this._viewableDownloadItems.set(downloadedItem.source.url, elem);
+
+      // Debounce incoming onDownloadAdded notifications


Probably note this is for startup where all existing downloads are added

Mardak · 2018-04-19T22:24:39Z

system-addon/lib/DownloadsManager.jsm

+
+    // Only get downloads within the time threshold specified and sort by recency
+    let downloads = [...this._viewableDownloadItems.values()]
+                      .filter(elem => elem.download.endTime > Date.now() - threshold)


nit: Date.now() - threshold defined outside of the callback.

Mardak · 2018-04-19T22:24:39Z

system-addon/lib/DownloadsManager.jsm

+    }
+  }
+
+  async getDownloads({threshold, numItems = this._viewableDownloadItems.size, onlyExists = false}) {


If threshold is required, probably make it as the first parameter and have the second the options object

Mardak · 2018-04-19T22:24:39Z

system-addon/lib/DownloadsManager.jsm

+    return results;
+  }
+
+  uninit() {


Probably want to cancel the timer to be clean

Mardak · 2018-04-19T22:24:39Z

system-addon/content-src/lib/link-menu-options.js

+      type: at.SHOW_DOWNLOAD_FILE,
+      data: {url: site.url}
+    }),
+    disabled: !site.path


I think we don't need this disabled anymore? Same for OpenFile

as-pine-proxy · 2018-04-20T03:19:26Z

Treeherder: 6da4e5b0e9c1f96de51c5c9c65234ffdf624159c

sarracini added the PR / In progress don't review yet label Apr 3, 2018

sarracini force-pushed the sarracini:bug_1433230 branch 2 times, most recently from 13e4d5f to 04dea6b Apr 5, 2018

sarracini requested a review from Mardak Apr 5, 2018

sarracini assigned Mardak Apr 5, 2018

sarracini added PR / Needs review and removed PR / In progress don't review yet labels Apr 5, 2018

sarracini reviewed Apr 5, 2018

View changes

sarracini force-pushed the sarracini:bug_1433230 branch from 78790cb to b6f938e Apr 5, 2018

sarracini reviewed Apr 5, 2018

View changes

sarracini changed the title ~~[WIP] Bug 1433230 - Add recent downloads to Highlights~~ Bug 1433230 - Add recent downloads to Highlights Apr 6, 2018

sarracini force-pushed the sarracini:bug_1433230 branch from b6f938e to cf47e83 Apr 9, 2018

sarracini added the Blocked label Apr 9, 2018

sarracini mentioned this pull request Apr 9, 2018

Fix Bug 1451091 - Chronologically order highlights #4085

Merged

sarracini removed the Blocked label Apr 9, 2018

sarracini force-pushed the sarracini:bug_1433230 branch from cf47e83 to de114d4 Apr 9, 2018

Mardak reviewed Apr 9, 2018

View changes

k88hudson reviewed Apr 10, 2018

View changes

Mardak suggested changes Apr 12, 2018

View changes

sarracini force-pushed the sarracini:bug_1433230 branch 4 times, most recently from 5879c98 to 2b9a547 Apr 12, 2018

sarracini force-pushed the sarracini:bug_1433230 branch 2 times, most recently from 4719ff6 to 4edbf4d Apr 16, 2018

sarracini changed the title ~~Bug 1433230 - Add recent downloads to Highlights~~ Fix Bug 1433230 - Add recent downloads to Highlights Apr 17, 2018

sarracini force-pushed the sarracini:bug_1433230 branch 2 times, most recently from c1c7763 to a71705a Apr 17, 2018

Mardak suggested changes Apr 18, 2018

View changes

Mardak assigned sarracini and unassigned Mardak Apr 18, 2018

sarracini force-pushed the sarracini:bug_1433230 branch from a71705a to 80ff8e2 Apr 19, 2018

sarracini force-pushed the sarracini:bug_1433230 branch from 80ff8e2 to e511e74 Apr 19, 2018

Mardak reviewed Apr 19, 2018

View changes

Mardak approved these changes Apr 19, 2018

View changes

Mardak added PR / R+ with nits and removed PR / Needs review labels Apr 19, 2018

sarracini force-pushed the sarracini:bug_1433230 branch from e511e74 to 7227655 Apr 19, 2018

Mardak reviewed Apr 19, 2018

View changes

sarracini force-pushed the sarracini:bug_1433230 branch from 7227655 to 2ed05ad Apr 19, 2018

Mardak reviewed Apr 19, 2018

View changes

Mardak mentioned this pull request Apr 19, 2018

Fix Bug 1432595 - Add section in prefs to turn off parts of Highlights #4111

Merged

Mardak approved these changes Apr 19, 2018

View changes

sarracini force-pushed the sarracini:bug_1433230 branch from 2ed05ad to 319c080 Apr 19, 2018

Fix Bug 1433230 - Add recent downloads to Highlights

31d91b1

sarracini force-pushed the sarracini:bug_1433230 branch from 319c080 to 31d91b1 Apr 19, 2018

Mardak merged commit e87a190 into mozilla:master Apr 20, 2018
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

as-pine-proxy added the pushed-to-pine label Apr 20, 2018

Mardak mentioned this pull request Apr 20, 2018

Merge latest strings from activity-stream changeset a772240e93f97e05886ca7b3d2e7ce58deb7ad0d mozilla/activity-stream-l10n#15

Merged

mozilla / activity-stream Archived

Fix Bug 1433230 - Add recent downloads to Highlights #4076

Fix Bug 1433230 - Add recent downloads to Highlights #4076

Conversation

sarracini commented Apr 3, 2018 • edited

This comment has been minimized.

sarracini Apr 5, 2018 Author Contributor

This comment has been minimized.

Mardak Apr 12, 2018 Member

This comment has been minimized.

sarracini Apr 5, 2018 Author Contributor

This comment has been minimized.

Mardak Apr 9, 2018 Member

This comment has been minimized.

sarracini Apr 5, 2018 • edited Author Contributor

Mardak left a comment

This comment has been minimized.

Mardak Apr 9, 2018 Member

This comment has been minimized.

sarracini Apr 10, 2018 Author Contributor

This comment has been minimized.

Mardak Apr 12, 2018 Member

This comment has been minimized.

Mardak Apr 9, 2018 Member

This comment has been minimized.

sarracini Apr 10, 2018 Author Contributor

This comment has been minimized.

Mardak Apr 9, 2018 Member

This comment has been minimized.

Mardak Apr 9, 2018 Member

This comment has been minimized.

Mardak Apr 9, 2018 Member

This comment has been minimized.

Mardak Apr 9, 2018 Member

This comment has been minimized.

Mardak Apr 12, 2018 Member

This comment has been minimized.

k88hudson Apr 10, 2018 Member

sarracini commented Apr 10, 2018

Mardak left a comment

This comment has been minimized.

Mardak Apr 12, 2018 Member

This comment has been minimized.

Mardak Apr 12, 2018 Member

This comment has been minimized.

sarracini Apr 12, 2018 Author Contributor

This comment has been minimized.

Mardak Apr 12, 2018 Member

This comment has been minimized.

Mardak Apr 12, 2018 Member

This comment has been minimized.

Mardak Apr 12, 2018 Member

This comment has been minimized.

Mardak Apr 12, 2018 Member

This comment has been minimized.

Mardak Apr 12, 2018 Member

This comment has been minimized.

sarracini Apr 12, 2018 Author Contributor

This comment has been minimized.

Mardak Apr 12, 2018 Member

This comment has been minimized.

Mardak Apr 12, 2018 Member

This comment has been minimized.

Mardak Apr 12, 2018 Member

This comment has been minimized.

Mardak Apr 12, 2018 Member

sarracini commented Apr 12, 2018

sarracini commented Apr 12, 2018

sarracini commented Apr 16, 2018

Mardak left a comment

This comment has been minimized.

Mardak Apr 18, 2018 Member

This comment has been minimized.

Mardak Apr 18, 2018 Member

This comment has been minimized.

sarracini Apr 19, 2018 Author Contributor

This comment has been minimized.

Mardak Apr 18, 2018 Member

This comment has been minimized.

Mardak Apr 18, 2018 Member

sarracini commented Apr 3, 2018 •

edited

sarracini Apr 5, 2018
Author Contributor

Mardak Apr 12, 2018
Member

sarracini Apr 5, 2018
Author Contributor

Mardak Apr 9, 2018
Member

sarracini Apr 5, 2018 •

edited

Author Contributor

Mardak Apr 9, 2018
Member

sarracini Apr 10, 2018
Author Contributor

Mardak Apr 12, 2018
Member

Mardak Apr 9, 2018
Member

sarracini Apr 10, 2018
Author Contributor

Mardak Apr 9, 2018
Member

Mardak Apr 9, 2018
Member

Mardak Apr 9, 2018
Member

Mardak Apr 9, 2018
Member

Mardak Apr 12, 2018
Member

k88hudson Apr 10, 2018
Member

Mardak Apr 12, 2018
Member

Mardak Apr 12, 2018
Member

sarracini Apr 12, 2018
Author Contributor

Mardak Apr 12, 2018
Member

Mardak Apr 12, 2018
Member

Mardak Apr 12, 2018
Member

Mardak Apr 12, 2018
Member

Mardak Apr 12, 2018
Member

sarracini Apr 12, 2018
Author Contributor

Mardak Apr 12, 2018
Member

Mardak Apr 12, 2018
Member

Mardak Apr 12, 2018
Member

Mardak Apr 12, 2018
Member

Mardak Apr 18, 2018
Member

Mardak Apr 18, 2018
Member

sarracini Apr 19, 2018
Author Contributor

Mardak Apr 18, 2018
Member

Mardak Apr 18, 2018
Member

Mardak Apr 18, 2018
Member

Mardak Apr 18, 2018
Member

Mardak Apr 18, 2018
Member

Mardak Apr 18, 2018
Member

sarracini Apr 19, 2018
Author Contributor

Mardak Apr 19, 2018
Member

Mardak Apr 19, 2018
Member

Mardak Apr 19, 2018
Member

Mardak Apr 19, 2018
Member

Mardak Apr 19, 2018
Member

Mardak Apr 19, 2018 •

edited

Member

Mardak Apr 19, 2018
Member

sarracini Apr 19, 2018
Author Contributor

Mardak Apr 19, 2018
Member

Mardak Apr 19, 2018
Member

Mardak Apr 19, 2018
Member

Mardak Apr 19, 2018
Member

Mardak Apr 19, 2018
Member

Mardak Apr 19, 2018
Member

Mardak Apr 19, 2018
Member

Mardak Apr 19, 2018
Member