Closed Bug 858155 Opened 10 years ago Closed 10 years ago

Auto-complete menus can span the entire screen

Categories

(Firefox for Metro Graveyard :: Input, defect)

x86_64
Windows 8.1
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jimm, Assigned: jimm)

Details

(Whiteboard: [forms])

Attachments

(2 files, 1 obsolete file)

related: bug 857986

See screenshot.
Both width and height are an issue.
Summary: auto-complete menu widths can span the entire screen → Auto-complete menus can span the entire screen
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → jmathies
Attachment #733508 - Flags: review?
Attachment #733508 - Attachment is obsolete: true
Attachment #733508 - Flags: review?
Attached patch patchSplinter Review
Comment on attachment 733514 [details] [diff] [review]
patch

what this does:

- restricts auto-completes to the width of the parent input
- restricts the height of auto-completes to a max of 350px.
- puts the left or right hand padding back on forcePosition menus, which got clipped out at some point.
Attachment #733514 - Flags: review?(fyan)
Comment on attachment 733514 [details] [diff] [review]
patch

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

Thanks!

::: browser/metro/base/content/helperui/MenuUI.js
@@ +48,2 @@
>      options.source = Ci.nsIDOMMouseEvent.MOZ_SOURCE_TOUCH;
>      return options;

Nit: Could you make this function body look like the following before landing?
function _positionOptions() {
  return {
    forcePosition: true;
    ...
    source: Ci.nsIDOMMouseEvent.MOZ_SOURCE_TOUCH;
  };
}

@@ +417,5 @@
> +    let leftHand = MetroUtils.handPreference == MetroUtils.handPreferenceLeft;
> +    if (aSource && aSource == Ci.nsIDOMMouseEvent.MOZ_SOURCE_TOUCH) {
> +      if (leftHand) {
> +        this._commands.setAttribute("left-hand", true);
> +        this._commands.removeAttribute("right-hand");

If this is a binary setting, could we change this to just use one attribute? (If that's the case, let's file a followup and deal with it later.)
Attachment #733514 - Flags: review?(fyan) → review+
Oops, I meant the following, not the semi-colons:

function _positionOptions() {
  return {
    forcePosition: true,
    ...
    source: Ci.nsIDOMMouseEvent.MOZ_SOURCE_TOUCH
  };
}
https://hg.mozilla.org/mozilla-central/rev/0a99f51a4d7c
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.