Closed Bug 554635 Opened 10 years ago Closed 10 years ago
Keystroke handling worked in earlier versions of FF but not in 3
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:220.127.116.11) Gecko/20100316 Firefox/3.6.2 (.NET CLR 3.5.30729) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:18.104.22.168) Gecko/20100316 Firefox/3.6.2 (.NET CLR 3.5.30729) First draw a few nodes (by clicking on the screen). Then hit the A key. In other browsers (Opera, IE/ASV, Safari, Chrome), all drawn nodes should be selected. Was working in FF in October 2009. (other keys suffer from the same problem -- t, delete, etc -- see help file for other options, though the shift key is working as it should -- shift-click adds or deletes from the selected collection. Reproducible: Always Steps to Reproduce: 1.see above 2. 3. Actual Results: see above Robert Longson and I talked a bit about this problem. He and Cameron probably saw the software in action at SVGOpen.
David. There are nightly builds here: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/ 3.6.x is equivalent to -mozilla-1.9.2 If you can download these nightlies and work out the day that it stopped working by using a binary chop then we're far more likely to be able to fix this.
Marking as new and broken on recent trunk. broken: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a4pre) Gecko/20100331 Minefield/3.7a4pre These are on mozilla-central. If you need a regression window for mozilla-1.9.2, add the keyword again and state so. Regression window: works: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090610 Minefield/3.6a1pre http://hg.mozilla.org/mozilla-central/rev/90d3e6d2cbb9 broken: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090611 Minefield/3.6a1pre http://hg.mozilla.org/mozilla-central/rev/4430cae50dad http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=90d3e6d2cbb9&tochange=4430cae50dad
Status: UNCONFIRMED → NEW
Ever confirmed: true
http://hg.mozilla.org/mozilla-central/rev/cabb8925dcd3 is the cause. http://hg.mozilla.org/mozilla-central/rev/771509552a2e is the last working build.
Component: General → XUL
Product: Firefox → Core
QA Contact: general → xptoolkit.widgets
The page only marginally works before the focus changes as well. It only does at first because the child page calls window.focus() upon loading. Focusing some other element will shift focus away and it won't work any more. Clicking the svg area does not shift the focus back to it. In fact, there is no means for the user to refocus the svg area once it's lost. The page is using an svg page loaded via an <object>, which seems the likely distinction here. Are the docshells hooked up the same for this? The focus manager relies on GetSubDocumentFor(iframeNode) to navigate down and window->GetFrameElementInternal() to go up. Presumably these aren't set for <object> elements?
> Are the docshells hooked up the same for this? Yes. > Presumably these aren't set for <object> elements? They should be for document-containing <object>s like this one.
OK, I'll do some further testing.
The regression here is caused because nsHTMLObjectElement::IsHTMLFocusable indicates false. This is because the tabindex defaults to -1 for <object> if it isn't specified, thus <object> cannot be focused. Note that when <object> contains a plugin, IsHTMLFocusable returns true unconditionally. Changing the implementation of nsHTMLObjectElement::GetTabIndex to default to 0 instead of -1 fixes the regression, and as a bonus makes tab navigation back to the svg frame work again. (Tabbing out doesn't work, but the page explicitly calls preventDefault on all keypresses) However, I'm not sure what side effects this patch might have.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Needed, but I don't think it blocks the next release. If we get a reviewed and baked patch, we'll approve it.
Attachment #439008 - Flags: review?(jonas) → review+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
blocking2.0: ? → beta1+
Priority: -- → P2
Removing the blocking flag as this wouldn't block a branch release. If you would like to nominate this for a branch, please make sure to have a roll-up patch with all changes.
blocking1.9.2: needed → -
Adding Jonas in CC (he did the review). The fix in this patch seems to be wrong. It has changed GetTabIndex to returns 0 in case of tabindex isn't specified thus making <object> always focusable. Actually, IsHTMLFocusable is completely broken and shouldn't look at GetTabIndex considering that GetTabIndex should return 0 or -1 as a default value depending of the focusable state of the element... At least two bugs have been reported because of that: bug 569315 and bug 596350p If Neil and Jonas agree, I will revert this change and add tests checking that, by default, an <object> isn't focusable. I will also open a follow up to clean the mess that IsHTMLFocusable is...
Can you explain the alternative way you plan on fixing this bug?
(In reply to comment #16) > Can you explain the alternative way you plan on fixing this bug? I have no idea how this should be fixed but having <object> focusable by default is clearly not the way to do it. Opera and Chromium do not have <object> focusable by default and this site works for them. I guess the issue must be somewhere else.
You need to log in before you can comment on or make changes to this bug.