Same as bug 64076, but for html form controls instead of prefs under Classic. Mozilla currently doesn't use the dotted border to indicate a focused listbox (the focused item within the listbox?), so it's difficult to see what you're doing on a page like http://bugzilla.mozilla.org/query.cgi when you're using the keyboard. See comments in bug 64076 for more details.

Futuring bug because of transition to XBL form controls

QA Contact Update

Added access and fcc508 keywords. Needed for legal compliance to FCC Section 508. Mozilla-based products need this to be used by the US government.

*** Bug 89466 has been marked as a duplicate of this bug. ***

summary of bug 89466 : select box with SIZE > 1 does not display the dotted line when it receives focus

go to http://home.nbci.com/search/power/form/0,179,home-0,00.html?fd.srch.pwr tab to the select box with title "Media Type". You will notice that the there is no indication of the control receiving focus. i.e.- There is no dotted line. But if you use the arrow keys, you will be able to navigate within the control and make your selection, too.

this is happening on all platforms

any progress with this bug?

When bug 40983 lands we're going to need this even more. CTRL+up/down will allow you to silently move the cursor (without changing the selection) up and down the box.

Bugs affecting accessibility are now get blocker status. Rods, please retarget to a milestone other than FUTURE. Can you give an ETA? Thanks - Aaron

This will be fixed when XBL select is turned on. XBL select is already in the tree and just needs the switch flipped when it's fully tested. I believe the XUL list control supports this. See bug 112713.

Right, but I've heard that XBL form controls are not 100% certain to make it into all embedding targets we care about.

bryner said he is working on getting the XBL framing in for select, but again, I cannot garuntee 100% that all embedding clients will use it. I'll try to get some specifics on what the clients want.

Extremely difficult, if not impossible, with the current implementation of the list box.

This will have to wait for XBL form controls. Marking nsbeta1-.

XBL form controls are nsbeta1+'d. also adding topembed keyword.

Marking nsbeta1+, topembed+, m1.0. Rod, can the outline be painted in the overlay layer after the content, as was done in bug 105742?

Based on comment #15, reassigning to byrner and removing the pluses from nsbeta1+ and topembed+. If XBL form controls are at risk, then we need some lead time to fix this.

That last comment should have read "based on comment #17"

There is some risk, since they didn't land in 0.9.9, and mozilla.org hasn't agreed to enable them by default yet, but as long as we now have good QA coverage on them, and people using them as dogfood, I think it still makes sense to use them as planned. Do we even have a fallback plan? I thought the current form controls hadn't been gettin' much lovin' lately...

nsbeta1+ per Nav triage team

Tested using the following: 3_22_05 netscape trunk 3_22_11 mfcembed trunk 3_22_11 mfcembed 0.9.9 branch with: http://bugzilla.mozilla.org/query.cgi Behavior same with all applications tested. On the query page there is no dotted line focus on the listboxes. For instance, on the six listboxes at the top (status, resolution, platformn OpSys, priority, severity), the tab key takes you through the boxes, but does not display with dotted borders. After tabbing out you get the dotted border again with the 'email' link.

Please update this bug with an [adt1] - [adt3] impact rating (or take it off the list if it doesn't even rate adt3.) Thanks!

This should be reassigned if the bug is to be fixed in the current(non-XBL) form controls.

rods, for retriage

Click on the checkbox to get focus to the right spot and then press <tab> to move between the listboxes.

This displays a focus ring when the listbox has focus for HTML listboxes (NOT XBL listboxes) The idea is that when the SelectAreaFrame (which is the parent) of the options is asked to paint then it asks the ListControlframe to paint the focus ring in the correct spot. It need to find the first non-disabled option (ignoring opt groups) and if nothing is selected it need to find the first frame which is the dummy option. Also, it now track thru a static data member which listControl frame currently has focus (this was taking from how comboboxes track it intnerally) With all the safety checks it should be low risk.

Comment on attachment 78384 [details] [diff] [review] patch sr=attinasi, assuming you tested all of the possible focus scenarios: actual selection, no selection, no selection with all disabled options, no options, etc.

New patch with jkeiser's suggestions for getting the <ctrl> arrow keys working.

Comment on attachment 78445 [details] [diff] [review] patch with <ctrl>arrow keys work. sr=attinasi

Comment on attachment 78445 [details] [diff] [review] patch with <ctrl>arrow keys work. >Index: html/forms/src/nsListControlFrame.cpp >=================================================================== >@@ -521,6 +526,164 @@ > > } > >+void nsListControlFrame::PaintFocus(nsIRenderingContext& aRC, nsFramePaintLayer aWhichLayer) >+{ >+ if (NS_FRAME_PAINT_LAYER_FOREGROUND != aWhichLayer) return; >+ >+ if (mFocused != this) return; >+ >+ PRInt32 selectedIndex = mEndSelectionIndex; >+ PRInt32 actualSelectedIndex = mEndSelectionIndex; >+ GetSelectedIndex(&actualSelectedIndex); >+ >+ if (selectedIndex == kNothingSelected) { >+ selectedIndex = actualSelectedIndex; >+ } selectedIndex isn't really selectedIndex, it's should be named focusedIndex; then actualSelectedIndex could be changed to selectedIndex. >+ nsIScrollableView * scrollableView; >+ GetScrollableView(scrollableView); >+ if (!scrollableView) return; >+ >+ nsCOMPtr<nsIContent> content; >+ >+ nsCOMPtr<nsIPresShell> presShell; >+ mPresContext->GetShell(getter_AddRefs(presShell)); >+ if (!presShell) return; >+ >+ nsIFrame* containerFrame; >+ GetOptionsContainer(mPresContext, &containerFrame); >+ if (!containerFrame) return; >+ >+ nsIFrame * childframe = nsnull; >+ nsresult result = NS_ERROR_FAILURE; >+ Move the definition of content here; consider calling it focusedContent, but not necessary. >+ // If we have a selected index then get that child frame >+ if (selectedIndex != kNothingSelected) { >+ content = getter_AddRefs(GetOptionContent(selectedIndex)); >+ if (content) { >+ // otherwise we find the content's frame and scroll to it >+ if (content) { Double if (content). >+ result = presShell->GetPrimaryFrameFor(content, &childframe); >+ } >+ } >+ } else { >+ // Since there isn't a selected item we need to show a focus ring around the first >+ // non-disabled item and skip all the option group elements (nodes) >+ nsCOMPtr<nsIDOMNode> node; >+ >+ nsCOMPtr<nsIDOMNSHTMLSelectElement> selectNSElement(do_QueryInterface(mContent)); >+ NS_ASSERTION(selectNSElement, "Can't be null"); >+ >+ nsCOMPtr<nsISelectElement> selectElement(do_QueryInterface(mContent)); >+ NS_ASSERTION(selectElement, "Can't be null"); >+ >+ nsCOMPtr<nsIDOMHTMLSelectElement> selectHTMLElement(do_QueryInterface(mContent)); >+ NS_ASSERTION(selectElement, "Can't be null"); >+ >+ PRUint32 length; >+ selectHTMLElement->GetLength(&length); >+ if (length) { >+ // find the first non-disabled item >+ PRBool isDisabled = PR_TRUE; >+ for (PRInt32 i=0;i<PRInt32(length) && isDisabled;i++) { >+ if (NS_FAILED(selectNSElement->Item(i, getter_AddRefs(node))) && !node) { >+ break; >+ } I think the && was meant to be ||. >+ if (NS_FAILED(selectElement->IsOptionDisabled(i, &isDisabled))) { >+ break; >+ } >+ if (isDisabled) { >+ node = nsnull; >+ } else { >+ break; >+ } >+ } >+ if (!node) { >+ return; >+ } >+ } >+ >+ // if w found a node use, if not get the first child (this is for empty selects) >+ if (node) { >+ nsCOMPtr<nsIContent> optContent(do_QueryInterface(node)); >+ result = presShell->GetPrimaryFrameFor(optContent, &childframe); I believe here you need to just use "content" so that it will be detected below, in the optgroup detection bit. Also what this means is the above code for (if (content) GetPrimaryFrameFor ...) can be moved down below, as can the GetPresContext and GetOptionsContainer (assuming next comment is right). >+ } >+ if (!childframe) { >+ result = containerFrame->FirstChild(mPresContext, nsnull, &childframe); I suspect we should not be doing this--is there a reason? If we couldn't find the first non-disabled option to focus, do we really want to focus the first childframe? Not only that, but I don't think we're guaranteed the first childframe is an option. It could be an optgroup, for example. >+ } >+ } >+ >+ if (NS_FAILED(result) || !childframe) return; >+ >+ const nsIView * clippedView; >+ scrollableView->GetClipView(&clippedView); >+ if (!clippedView) return; >+ >+ nscoord x; >+ nscoord y; >+ scrollableView->GetScrollPosition(x,y); >+ // get the clipped rect >+ nsRect rect; >+ clippedView->GetBounds(rect); >+ >+ // now move it by the offset of the scroll position >+ rect.x = 0; >+ rect.y = 0; >+ rect.MoveBy(x,y); >+ >+ // get the child rect >+ nsRect fRect; >+ childframe->GetRect(fRect); >+ >+ float p2t; >+ mPresContext->GetScaledPixelsToTwips(&p2t); >+ nscoord onePixelInTwips = NSToCoordRound(p2t); >+ Please move this closer to where it's used ... >+ nsRect clipRect; >+ containerFrame->GetRect(clipRect); >+ clipRect.x = 0; >+ clipRect.y = 0; >+ >+ PRBool clipEmpty; >+ aRC.PushState(); >+ aRC.SetClipRect(clipRect, nsClipCombine_kReplace, clipEmpty); >+ >+ // adjust position is it is a child of a option group >+ if (content) { >+ nsRect optRect(0,0,0,0); >+ nsCOMPtr<nsIContent> parentContent; >+ content->GetParent(*getter_AddRefs(parentContent)); >+ nsCOMPtr<nsIDOMHTMLOptGroupElement> optGroup(do_QueryInterface(parentContent)); >+ if (optGroup) { >+ nsIFrame * optFrame; >+ result = presShell->GetPrimaryFrameFor(parentContent, &optFrame); >+ if (NS_SUCCEEDED(result) && optFrame) { >+ optFrame->GetRect(optRect); >+ } >+ } >+ fRect.y += optRect.y; >+ } >+ >+ // set up back stop colors and then ask L&F service for the real colors >+ nscolor color = selectedIndex >= 0?NS_RGB(245,219,149):NS_RGB(0,0,0); Por favor, space out the ? and : a bit ... it's hard to pick out. >+ nsCOMPtr<nsILookAndFeel> lookAndFeel; >+ mPresContext->GetLookAndFeel(getter_AddRefs(lookAndFeel)); >+ if (actualSelectedIndex == kNothingSelected) { >+ lookAndFeel->GetColor(nsILookAndFeel::eColor_WidgetSelectBackground, color); >+ } else if (lookAndFeel) { >+ lookAndFeel->GetColor(selectedIndex >= 0?nsILookAndFeel::eColor_WidgetSelectForeground:nsILookAndFeel::eColor_WidgetSelectBackground, color); >+ } >+ >+ nsRect dirty; >+ nscolor colors[] = {color, color, color, color}; >+ PRUint8 borderStyle[] = {NS_STYLE_BORDER_STYLE_DOTTED, NS_STYLE_BORDER_STYLE_DOTTED, NS_STYLE_BORDER_STYLE_DOTTED, NS_STYLE_BORDER_STYLE_DOTTED}; >+ nsRect innerRect = fRect; >+ innerRect.Deflate(nsSize(onePixelInTwips, onePixelInTwips)); >+ nsCSSRendering::DrawDashedSides(0, aRC, dirty, borderStyle, colors, fRect, innerRect, 0, nsnull); >+ >+ aRC.PopState(clipEmpty); >+ >+} >+ > //--------------------------------------------------------- > // Frames are not refcounted, no need to AddRef > NS_IMETHODIMP >@@ -1823,6 +1986,18 @@ > nsListControlFrame::SetFocus(PRBool aOn, PRBool aRepaint) > { > // XXX:TODO Make set focus work This is no longer needed, I think you just fixed all our focus problems :) Fix these and r=jkeiser

Made John's fixs up (mostly cosmetic)

Comment on attachment 78485 [details] [diff] [review] patch r=jkeiser Carrying sr=attinasi as well

What testing have you done on this to ensure it doesn't not cause any regressions in perf, stability or layout? Removing adt1.0.0. Pls land this on the trunk, and let it bake for a couple of days. If there are no regressions, and QA approves, pls renominate for ADT approval.

checked into trunk

Comment on attachment 78485 [details] [diff] [review] patch a=asa (on behalf of drivers) for checkin to the 1.0 branch

Verifying fixed on trunk: build 2002-04-12-03-trunk windows 98 and build 2002-04-12-03-trunk MacOSX

checkin for this bug has resulted in bug 137341

Backed out the one lnie that was causing this to happen.

fixed

please verify this again with the one line that was backed out from the trunk to fix the new regression.

terri, please verify this bug is fixed in the trunk. The adt needs to know it's tested before the fix is checked into the branch. thanks!

need to reopen doesn't work for mulitple selections

The previous patch didn't properly higfhlight selects with more than one item selected.

This is more streamline, it only gets the selected index if mEndSelectedIndex is kNothingSelected

Comment on attachment 79533 [details] [diff] [review] better patch + nsCOMPtr<nsIDOMNSHTMLSelectElement> selectNSElement(do_QueryInterface(mContent)); + NS_ASSERTION(selectNSElement, "Can't be null"); + + nsCOMPtr<nsISelectElement> selectElement(do_QueryInterface(mContent)); + NS_ASSERTION(selectElement, "Can't be null"); + If you could move these into the if() it would be nice. Veeeeery nice patch :) r=jkeiser

Comment on attachment 79533 [details] [diff] [review] better patch sr=attinasi

We have found now "hangs" with Win2k or Linux with today's build. fixed

I can't get this to hang on Win2k or Linux. - fixed

When selecting multiple items by dragging you can see two bugs: when selecting downward the first item gets the focus, not the last. And when selecting upwards all of the items get the focus. I reported a separate bug 137588 about this problem. Other then that the problem is fixed. Verifying on trunk builds 2002-04-16-03-trunk on Win2k, 2002-04-17-07-trunk on Linux RedHat, and build 2002-04-17-03-trunk on OSX

vfy

adt1.0.0+ (on ADT's behalf) approval for checking into the 1.0 branch. Pls check this in today, then add the fixed1.0.0 keyword.

fixed on branch

verifying on branch from 07/17 on windows98 and linux RedHat