…when it lands, that is.

"Selections" mean both "clicks on" or "enter keypresses when focused". Also, be sure to log the type of selection (enter key or mouse click).

This patch depends on the one for bug 1036908 to provide a bunch of the common code.

(Hey Petruta, I've assigned this bug to you because it's so similar to bug 1036908. Feel free to re-assign it if you want. :) To test this change: Open a new tab Make sure the search box is using Google. Type "test". Hit enter. Open about:telemetry in a new tab. Expand the "Simple Measurements" section. Look for the "UITelemetry" key. Make sure it contains: "search":{"newtab":1} Open a new tab Type "test" Hit arrow-down, then enter. Go back to about:telemetry Refresh the page Re-expand the "Simple Measurements" section. Make sure the "UITelemetry" key now contains: "search":{"newtab":2,"selection":{"newtab":{"0":{"key":1}}}} Open a new tab Type "test" Click on the first entry Go back to about:telemetry Refresh the page Re-expand the "Simple Measurements" section. Make sure the "UITelemetry" key now contains: "search":{"newtab":3,"selection":{"newtab":{"0":{"key":1,"mouse":1}}}} Open a new tab Type "test" Click on the second entry Go back to about:telemetry Refresh the page Re-expand the "Simple Measurements" section. Make sure the "UITelemetry" key now contains: "search":{"newtab":4,"selection":{"newtab":{"0":{"key":1,"mouse":1},"1":{"mouse":1}}}}

Comment on attachment 8484208 [details] [diff] [review] Log search suggestions in about:newtab. Review of attachment 8484208 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/newtab/search.js @@ +44,5 @@ > searchString: searchStr, > whence: "newtab", > + } > + > + if (selectionIndex && selectionIndex != "-1") { my comment is basically the same as for Bug 1036908, the selectionindex check is wrong, and -1 could "probably" be avoided by not setting it in the contentsearch at all.

Fixed the selectionIndex check to match the other bug. :)

Comment on attachment 8484552 [details] [diff] [review] The next iteration. Review of attachment 8484552 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/newtab/search.js @@ +44,5 @@ > searchString: searchStr, > whence: "newtab", > + } > + > + if (selectionIndex !== "") { same comment about using hasAttribute I made in the other bug. Apart that, lgtm!

And the same change made as in the other bug.

sorry had to backout this changes (for Bug 1036914, Bug 1036912 and Bug 1036908) since one of this changes caused test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=47585310&tree=Fx-Team

Looks like this was it (by inspection)

Try run at https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8be8501f1bf2 If it's successful, I'll re-ask for checkin. (Although, I'm getting a lot of seemingly-unrelated errors from a run with the previous version of the patch at https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=14f042ccc93d so I'll probably re-ask for checkin if it fails and I can't see any related errors…)

Okay, given the green try-run at https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8be8501f1bf2 , I'm re-requesting checkin. Thanks!

Thanks for the clear steps, Blake! I've obtained the results specified in comment 3 using Nightly 35.0a1 20140912030202 under Win 7 64-bit, Ubuntu 12.10 32-bit and Mac OSX 10.9.5.

You're very welcome, Petruta! It seemed like a hard thing to test if you didn't know what the code was trying to do… :)