Detail view blocks scrolling with page-up, page-down, and arrow keys

RESOLVED FIXED in mozilla15

Status

()

RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: bj, Assigned: angietngo)

Tracking

({access})

Trunk
mozilla15
access
Points:
---
Bug Flags:
in-testsuite -
in-moztrap -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [AOMTestday][good first bug][mentor=bmcbride@mozilla.com][fixed-in-fx-team])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:2.0b8pre) Gecko/20101202 Firefox/4.0b8pre
Build Identifier: 4.0b8pre (installed from nightly on 3 December 2010)

When viewing the description of an installed add-on, the page-up and page-down keys should scroll the displayed description. The keys don't work when you first view a description.

Note: If you press the tab key or any arrow key then the page up and page down keys start working.

Reproducible: Always

Steps to Reproduce:
1.Install LavaFox V1, or other add-on with a long description.
2.Open Add-ons Manager.
3.Click Appearance/Lavafox/More.
4.Read visible text.
5.Press "Page Down" key.
6.Press "Page Down" key several more times in frustration.
7.Check whether your system is hung.
Actual Results:  
Your system is not hung and the first page of the LavaFox description is still visible.

Expected Results:  
Either your system is hung or the last page of the LavaFox description is visible.
(Reporter)

Updated

8 years ago
Whiteboard: [AOMTestday]
Version: 1.9.2 Branch → Trunk
That's interesting. When you enter the details view all arrow keys don't work either. You will have to select a control in that view first, before scrolling works.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Add-ons Manager: Detailed display for add-on should scroll with page-up and page-down keys → Detail view blocks scrolling with page-up, page-down, and arrow keys

Updated

8 years ago
Keywords: access

Updated

8 years ago
Blocks: 563909
(Reporter)

Comment 2

8 years ago
(In reply to comment #1)
> That's interesting. When you enter the details view all arrow keys don't work
> either. You will have to select a control in that view first, before scrolling
> works.

Actually, the down arrow key works, but it only has effect the third time it is pressed. The first two times have no visible effect. (Same results on Ubuntu and Windows 7.) I'm going to add another bug report for that behavior.
Likely we just need to ensure the scrollable area gets focus for this to work.
Whiteboard: [AOMTestday] → [AOMTestday][good first bug]
Duplicate of this bug: 616463
Duplicate of this bug: 708985

Comment 6

7 years ago
(In reply to Dave Townsend (:Mossop) from comment #3)
> Likely we just need to ensure the scrollable area gets focus for this to
> work.

Yes. As I wrote in the duplicate bug I filed - the keyboard focus stays on the (no longer visible) add-on list. You can press Down a few times and then Enter, it will open the details page for a different add-on.
Confirming info from comment #3 and comment #6. This bug was triggering an issue in my Addons Manager Hilite extension; so I included a fix in version 1.0 (currently on the dev channel). Here's the code I used, in case it's helpful:

var dv = AMdoc.getElementById("detail-view");  //AMdoc is the document obj of the AOM
dv.setAttribute('tabindex', 0); 
dv.focus();
Assignee: nobody → angietngo
Status: NEW → ASSIGNED
Whiteboard: [AOMTestday][good first bug] → [AOMTestday][good first bug][mentor=bmcbride@mozilla.com]
(Assignee)

Comment 8

7 years ago
I need some help on pin pointing where to correct the bug.  I believe I am in the correct file in ...\mozilla-central\toolkit\mozapps\extensions\content\extensions.js.

I am currently looking at scroll preference (Lines: 2927-29141).  Do I focus on the BoxObject?  I was reading on BoxObject (https://developer.mozilla.org/en/XUL_Tutorial/Box_Objects) and seems feasible but when I add detailViewBoxObject.focus() it does not work.

I may need a little nudge to get me on the right track.

Comment 9

7 years ago
You probably want to add |this.currentViewObj.node.focus()| at the end of gViewController.loadViewInternal() - always focus a view when it loads rather than implementing a special case for the details view.

Comment 10

7 years ago
I am working with Angie on this bug and I tried the suggestion in comment #9 with no luck, do we have to blur the previous view before we focus the current view at the end of the method?
(In reply to Rich from comment #10)
> do we have to blur the previous view before we focus the
> current view at the end of the method?

No, just calling .focus() will automatically blur the previously focused element.

However, as comment 7 suggests, you will need to add a tabindex="0" attribute to the detail's view container element (id="details-view") in extensions.xul, which allows that element to gain focus.
See https://developer.mozilla.org/en/Accessibility/Keyboard-navigable_JavaScript_widgets for an explanation on how that works.

You should also add that attribute to the container elements for the other views also (ie, all children of the view-port <deck> element).
(In reply to Wladimir Palant from comment #9)
> You probably want to add |this.currentViewObj.node.focus()| at the end of
> gViewController.loadViewInternal() - always focus a view when it loads
> rather than implementing a special case for the details view.

Ideally, the element should be focused *before* the call to .show() (or .refresh()), so that a view's .show() method can change which element gets focus.
(Assignee)

Comment 13

7 years ago
Created attachment 620576 [details] [diff] [review]
changes made to extensions.js & extensions.xul to focus the add-on description box
Attachment #620576 - Flags: review?(bmcbride)
Comment on attachment 620576 [details] [diff] [review]
changes made to extensions.js & extensions.xul to focus the add-on description box

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

Thanks - this works good :) Just a couple of things to fix up:

::: toolkit/mozapps/extensions/content/extensions.js
@@ +645,5 @@
>        this.currentViewObj.refresh(view.param, ++this.currentViewRequest, aState);
>      else
>        this.currentViewObj.show(view.param, ++this.currentViewRequest, aState);
> +	  
> +	this.currentViewObj.node.setAttribute('tabindex', 0);  

This isn't needed, since you've already set this attribute for the various elements in extensions.xul

@@ +646,5 @@
>      else
>        this.currentViewObj.show(view.param, ++this.currentViewRequest, aState);
> +	  
> +	this.currentViewObj.node.setAttribute('tabindex', 0);  
> +	this.currentViewObj.node.focus();

Move this up a few lines, so it's directly after the following line:
  this.viewPort.selectedPanel.setAttribute("loading", "true");
(See comment 12 for why I'd like this.)

Also, you've used a tab to indent this line, but we use 2 spaces for indentation.
See https://developer.mozilla.org/En/Mozilla_Coding_Style_Guide for general code style hints.
Attachment #620576 - Flags: review?(bmcbride) → review-

Comment 15

7 years ago
Created attachment 621365 [details] [diff] [review]
first patch with suggested changes applied
Attachment #621365 - Flags: review?(bmcbride)
Attachment #620576 - Attachment is obsolete: true
Comment on attachment 621365 [details] [diff] [review]
first patch with suggested changes applied

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

Great - thanks Angie and Rich :)

I'll land this for you within the next couple of days.
Attachment #621365 - Flags: review?(bmcbride) → review+
https://hg.mozilla.org/integration/fx-team/rev/85c4026e625d
Flags: in-testsuite-
Flags: in-moztrap-
Flags: in-litmus?
Whiteboard: [AOMTestday][good first bug][mentor=bmcbride@mozilla.com] → [AOMTestday][good first bug][mentor=bmcbride@mozilla.com][fixed-in-fx-team]
Target Milestone: --- → mozilla15
you're in mozilla-central!

https://hg.mozilla.org/mozilla-central/rev/85c4026e625d
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Flags: in-litmus?
You need to log in before you can comment on or make changes to this bug.