Closed Bug 670317 Opened 8 years ago Closed 8 years ago
"ASSERTION: JS failed without setting an exception!" with set
We end up with that JS error because nsHTMLLegendElement::Focus calls nsFocusManager::MoveFocus which calls nsFocusManager::DetermineElementToMoveFocus which tries to get the next tabbable content, which ends up calling nsXULElement::IsFocusable on a <xul:textbox>. This is presumably happening because MoveFocus is used by the legend element to move focus to the next thing in tab order, but there's nothing else in the content document in tab order, so we end up trying to move the focus to chrome. This is probably buggy; enn, is there a way to restrict the focus move to the content document? That said, it's odd that trying to call back out through XPConnect there does a security check...
Should be fairly easy to add. Just implement an extra bitflag passed to MoveFocus. Then when set, DetermineElementToMoveFocus would need to return early here: http://mxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp#2517 (before where it says 'Traverse up to the parent document') I assume you still want to traverse down into subframes, but this will stop traversal up and to siblings.
There is in fact a security check: does that save us enough or do you think there's still a potential security problem here. Neil: would such a check interfere with tabbing around a multi-frame document, or just automatic focus moves as in this legend case?
Assignee: nobody → enndeakin
Whiteboard: [need answer to comment from 4 neil]
The intent would be that the flag would only be supplied by the call from nsHTMLLegendElement::Focus.
Whiteboard: [need answer to comment from 4 neil] → [sg:moderate][need answer to comment from 4 neil]
Neil - any idea when you'll be able to work on this?
Neil - did you mean to request review here?
Whiteboard: [sg:moderate][need answer to comment from 4 neil] → [sg:moderate]
Comment on attachment 591169 [details] [diff] [review] Add flag for legend focus Can do, I was waiting to ensure that tests passed which they do.
Attachment #591169 - Flags: review?(bugs)
Comment on attachment 591169 [details] [diff] [review] Add flag for legend focus I guess this is ok.
Attachment #591169 - Flags: review?(bugs) → review+
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
We should be able to do without fixing this on the ESR because the existing security check does prevent bad things happening.
Whiteboard: [sg:moderate] → [sg:moderate][qa+]
Whiteboard: [sg:moderate][qa+] → [sg:moderate][qa+][advisory-tracking+]
Whiteboard: [sg:moderate][qa!][advisory-tracking+] → [qa!][advisory-tracking+]
You need to log in before you can comment on or make changes to this bug.