The spec says to only draw the focus ring when one should be drawn on the element passed in. Yet, the current code only checks if it is focused. The check should also be checking if nsPIDOMWindow::ShouldShowFocusRing() returns true.

I'm trying to push a try build but the servers are stuck again.

Comment on attachment 8415979 [details] [diff] [review] Added code to check if the window should draw a focus ring. > nsIFocusManager* fm = nsFocusManager::GetFocusManager(); >- if (fm) { >+ if (fm && mCanvasElement) { > // check that the element i focused > nsCOMPtr<nsIDOMElement> focusedElement; > fm->GetFocusedElement(getter_AddRefs(focusedElement)); > if (SameCOMIdentity(aElement.AsDOMNode(), focusedElement)) { >- return true; >+ nsPIDOMWindow *window = mCanvasElement->OwnerDoc()->GetWindow(); >+ if (window) { >+ return window->ShouldShowFocusRing(); >+ } I know that you've checked that aElement is a descendant of the canvas, but it would be clearer if you used aElement->OwnerDoc()->GetWindow() here, and remove the null check earlier. You may find that some tests will now fail as the ring is no longer being shown unconditionally. You can simulate a keypress if needed.

try build: https://tbpl.mozilla.org/?tree=Try&rev=86bfd43c9d2c

sorry had to backout this change for causing bustage like https://tbpl.mozilla.org/php/getParsedLog.php?id=38923460&tree=Mozilla-Inbound

mozilla::dom::Element& aElement. Probably should be aElement.OwnerDoc()?

(In reply to Nochum Sossonko [:Natch] from comment #8) > mozilla::dom::Element& aElement. Probably should be aElement.OwnerDoc()? yes. sorry about that! Was trying to be too quick