Closed
Bug 616440
Opened 14 years ago
Closed 13 years ago
Detail view blocks scrolling with page-up, page-down, and arrow keys
Categories
(Toolkit :: Add-ons Manager, defect)
Toolkit
Add-ons Manager
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: bj, Assigned: angietngo)
References
Details
(Keywords: access, Whiteboard: [AOMTestday][good first bug][mentor=bmcbride@mozilla.com][fixed-in-fx-team])
Attachments
(1 file, 1 obsolete file)
5.79 KB,
patch
|
Unfocused
:
review+
|
Details | Diff | Splinter Review |
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•14 years ago
|
Whiteboard: [AOMTestday]
Version: 1.9.2 Branch → Trunk
Comment 1•14 years ago
|
||
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
Reporter | ||
Comment 2•14 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.
Comment 3•14 years ago
|
||
Likely we just need to ensure the scrollable area gets focus for this to work.
Whiteboard: [AOMTestday] → [AOMTestday][good first bug]
Comment 6•13 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.
Comment 7•13 years ago
|
||
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();
Updated•13 years ago
|
Assignee: nobody → angietngo
Status: NEW → ASSIGNED
Whiteboard: [AOMTestday][good first bug] → [AOMTestday][good first bug][mentor=bmcbride@mozilla.com]
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•13 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•13 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?
Comment 11•13 years ago
|
||
(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).
Comment 12•13 years ago
|
||
(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•13 years ago
|
||
Attachment #620576 -
Flags: review?(bmcbride)
Comment 14•13 years ago
|
||
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•13 years ago
|
||
Attachment #621365 -
Flags: review?(bmcbride)
Updated•13 years ago
|
Attachment #620576 -
Attachment is obsolete: true
Comment 16•13 years ago
|
||
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+
Comment 17•13 years ago
|
||
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
Comment 18•13 years ago
|
||
you're in mozilla-central!
https://hg.mozilla.org/mozilla-central/rev/85c4026e625d
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Flags: in-litmus?
You need to log in
before you can comment on or make changes to this bug.
Description
•