+++ This bug was initially created as a clone of Bug #801121 +++ Some testing on this slate shows that autocomplete is not implemented, though a suggestion box does appear for the bug summary

I think we're tracking stories vs bugs separately, re-opening.

If there is no existing bug for the work, then we can do that work in a "story" bug. If existing bugs have useful context, marking them as blocking a story seemed easier.

Before a review I just wanted you to take a look. Before this patch, earlier this week, I was doing a completely different approach similar to how desktop autocomplete works. But I think we want to handle touch and mouse input the same for a consistent look. Info about the patch: - The extra prefs are needed because otherwise an exception is thrown when there is no default value causing the autocomplete popup to not show up. I took the values from mobile/xul. - I added code to allow pen/mouse for form helper UI as well. - The removed block of code seems to be not needed and causes the autocomplete to not show up when it should. This functionality matches desktop. - The WantTypeBehind stuff is needed or else the autocomplete popup consumes the typing you do and causes it to not work right. The only problem I know of is that the autocomplete is shown when you first click. I'd appreciate any suggestions on the best way you think this should be handled. If you could, please apply the patch and let me know your thoughts.

Oh I should also mention that this patch also has a side effect for <select> elements. With -metrodesktop a mouse click on a <<select> causes the widget popup and the Menu UI popup to both be shown. But in WinRT it only shows the Menu UI one. If you think it's best we could also disable popups on the win32 widget backend if -metrodesktop is specified so it works in the same way with WinRT and -metrodesktop. This side effect is a net positive in WinRT since you can't use <select> elements at all at the moment.

Comment on attachment 709079 [details] [diff] [review] Patch v1. Review of attachment 709079 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. ::: browser/metro/base/content/contenthandlers/Content.js @@ +407,1 @@ > return; Can we just remove this whole if statement? ::: browser/metro/base/content/helperui/MenuUI.js @@ +289,5 @@ > MenuPopup.prototype = { > get _visible() { return !this._panel.hidden; }, > get _commands() { return this._popup.childNodes[0]; }, > + get WantTypeBehind () { return this._wantTypeBehind; }, > + set WantTypeBehind (aTypeBehind) { this._wantTypeBehind = aTypeBehind; }, Just access the property directly rather than through a getter and setter, please. (We can consider all the classes within a file "friends", and not worry about public/private API usage between them.) [Nit: If we were keeping the getter/setter, it should use a lowercase identifier.] ::: browser/metro/profile/metro.js @@ +385,5 @@ > > +// don't zoom to fit by less than 1/9 (11%) > +pref("browser.ui.zoom.pageFitGranularity", 9); > +pref("browser.ui.zoom.animationDuration", 200); > + This was my bad. You can drop this change now. I fixed it in: http://hg.mozilla.org/projects/elm/rev/9032a0b263ec

Everything is working great now, but please let me know i you have any suggestions.

Comment on attachment 709216 [details] [diff] [review] Patch v2. Review of attachment 709216 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. I noticed some minor autocomplete bugs while testing this, but they seem unrelated to this patch. ::: browser/metro/base/content/contenthandlers/Content.js @@ +427,5 @@ > + !(element instanceof HTMLSelectElement)) { > + this.lastClickElement = element; > + return; > + } > + this.lastClickElement = element; Can you rewrite this (starting from the "if" statement) as: this.lastClickElement = element; if (/* ... */) return;

Editing in-testsuite flag to check if this has tests. Otherwise, we'll mark for in-qa-testsuite for triage to see if we can make a Mozmill test for it.