Richard Corbin Reporter
	Description • 20 years ago

The dropdown boxes to choose the headers & footers combo in the 'File > Page
Setup ' dialog do not support the use of the Home, End, Page Up / PgUp, and Page
Down / PgDn keys for quick usage.

Steps to reproduce:
1. File > Page Setup
2. Select Margins & Header/Footer tab
3. In the 'Headers & Footers' section select a dropdown.
4a. Use End key
4b. Use Home key
4c. Use Page Down key
4d. Use Page Up key

Actual Results:
4a,b,c,d. Nothing happens

Expected Results:
4a. Taken to End/Bottom of dropdown list
4b. Taken to Home/Top of dropdown list
4c. Taken down a page in dropdown list (in this case, due to small dropdown
should be taken to bottom of list)
4d. Taken up a page in dropdown list (in this case, due to small dropdown should
be taken to top of list)


Using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040704
Firefox/0.9.0+

	Richard Corbin Reporter
	Comment 1 • 20 years ago

Apologies, step 3 should read:

3. In the 'Headers & Footers' tab to select a dropdown.

Stick to keyboard. (If you click with mouse the dropdown opens and then the
Home/End/PgUp/PgDn keys work).

	Aaron Leventhal
	Comment 2 • 20 years ago

- True for all combo boxes in Mozilla
- Home and End do currently work when the dropdown is open, but not when it's closed
- PageUp/PageDown never work

	Richard Corbin Reporter
	Comment 3 • 19 years ago

"Home" and "End" now working for me in Firefox 1.5 beta 1.

Not sure if PageUp and PageDn ever need to be fixed?

	Aaron Leventhal
	Comment 4 • 19 years ago

Everything is working for me now except pageup/pagedown in a closed list. They
work when the list is dropped down.

	Przemyslaw Bialik
	Comment 5 • 17 years ago

Adjusted description to reflect current state of this bug.

	Aaron Leventhal
	Comment 6 • 17 years ago

My comment 4 is wrong, it's broken in open menulists as well

	Neil Deakin Assignee
	Comment 7 • 17 years ago

What would pageup/pagedown do? Go to the start/end of the list?

	Aaron Leventhal
	Comment 8 • 17 years ago

I suggest looking at the code for the HTML combo and/or playing with the implementation in combo with a lot of items to see how it works.

The impl is based on what the combo box would look like if it were open. The first page up or dn goes to the first or last visible item at the current time. If you're already at the top or bottom of the visible list, the next one moves by the number of rows in the popup.

	Jim Mathies [:jimm]
	Comment 9 • 9 years ago

This is a chrome bug.

	Jim Mathies [:jimm]
	Comment 10 • 9 years ago

This is chrome bug that break the current (new) way we use popups in e10s, hence it blocks the project.

	neil@parkwaycc.co.uk
	Comment 13 • 9 years ago

Comment on attachment 8602200 [details] [diff] [review]
Patch with fixed up coordinate issues

>+    // Only scroll by page within menulists.
Would be nice if this worked for closed menulists too, but I guess we don't know the height?

>+  nsMenuFrame* newMenu = NULL;
Nit: nullptr

>+    // Get the position of the current item and add or subtract one popup's height
>+    // to or from it.
I'm worried that this makes too many assumptions about the layout. Would it be possible to use the scroll frame's height instead?

>+    // Look for the next child which is just past the target position. This child
>+    // will need to be selected.
The <select> code looks as if it selects the last visible child, rather than the first invisible child. (So for example, after pressing Page Up, pressing a single Page Down selects the last visible item, and then the next Page Down scrolls.) This is probably easily fixed by setting newMenu after the break condition.

r=me with the above fixed.

	Neil Deakin Assignee
	Comment 14 • 9 years ago

> Would be nice if this worked for closed menulists too, but I guess we don't know the height?

We do know the height since menulists have sizetopopup set, but we would need to update the menulist value as well. This can be a separate bug.

> This is probably easily fixed by setting newMenu after the break condition.

I added some additional code to handle when newMenu is disabled, so that we skip over any disabled items at the end of the page.

	neil@parkwaycc.co.uk
	Comment 15 • 9 years ago

(In reply to Neil Deakin from comment #14)
> > Would be nice if this worked for closed menulists too, but I guess we don't know the height?
> 
> We do know the height since menulists have sizetopopup set

When I tried the following code:
@@ -106,2 +106,8 @@ bool MenuBoxObject::HandleKeyPress(Keybo
   switch (keyCode) {
+    case nsIDOMKeyEvent::DOM_VK_PAGE_UP:
+    case nsIDOMKeyEvent::DOM_VK_PAGE_DOWN:
+    {
+      popupFrame->ChangeByPage(keyCode == nsIDOMKeyEvent::DOM_VK_PAGE_UP);
+      return true;
+    }
     case nsIDOMKeyEvent::DOM_VK_UP:
Page Up and Page Down simply selected the first or last item unless the menu had been open at least once. Perhaps it needs a call to SetPopupPosition?

> but we would need to update the menulist value as well.
Code for that already exists in menulist.xml

> I added some additional code to handle when newMenu is disabled, so that we
> skip over any disabled items at the end of the page.
Seems to work OK.

	neil@parkwaycc.co.uk
	Comment 16 • 9 years ago

Comment on attachment 8604821 [details] [diff] [review]
menulistbypage

>+    nsIScrollableFrame *scrollframe = do_QueryFrame(nsBox::GetChildBox(this));
Please could you use GetScrollFrame(this) instead? It works in an edge case where this code doesn't.

	Pulsebot
	Comment 17 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/d192c8d0ddb6

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 18 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/d192c8d0ddb6