Closed
Bug 419059
Opened 16 years ago
Closed 15 years ago
contentAccess accesskeys for elements hidden with CSS don't work
Categories
(Core :: DOM: Events, defect)
Core
DOM: Events
Tracking
()
RESOLVED
FIXED
People
(Reporter: jhsoby, Assigned: smaug)
References
Details
(Keywords: fixed1.9.1, regression, testcase)
Attachments
(2 files, 1 obsolete file)
386 bytes,
text/html
|
Details | |
4.87 KB,
patch
|
neil
:
review+
roc
:
superreview+
jst
:
approval1.9.1+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; nb-NO; rv:1.9b3) Gecko/2008020513 Firefox/3.0b3 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; nb-NO; rv:1.9b3) Gecko/2008020513 Firefox/3.0b3 Hi, and thanks for all of this, first of all. In Firefox 2, I was able to hide a link using CSS (display:hidden), but still activate it by pressing its accesskey combination. Because of this feature I was able to hide many of the layout elements in Wikipedia that I use often (e.g. Alt+X for Random page) [1], thus using the screen space more efficiently. WIth Gran Paradiso, however, that’s not possible any more. There might be an element in about:config that changes this behaviour, but so far I haven’t found anything. I’ve tried googling for this as well without finding anything relevant. [1] Yes, I’ve customized the ui.key.chromeAccess and ui.key.contentAccess behaviour to behave as it should. ;-) So alt instead of alt+shift. Reproducible: Always Steps to Reproduce: 1. Create an HTML document linking to some page, and make the link have accesskey 'c', but also hide the element using style="display:none;" 2.Then view the website, try Alt+C (or Alt+Shift+C). Actual Results: In Firefox 2, the link is loaded, in Firefox 3 nothing happens. Expected Results: Load the link. I would be happy if there was just an element in about:config to change to fix it. But as far as I can see, that doesn’t exist (yet).
We are experiences the same problem on our forum ( http://gathering.tweakers.net ) where we make extensive use of accesskeys and have means for the user to assign and change them as they like (thus also mitigating any conflict issues with chrome accelerator keys). Some of these accesskeys are assigned to visible links in the main menu, some however are assigned to links that are in submenus that are only visible when the users hovers over the corresponding main menu item. These links don't get triggered anymore in Firefox 3 when using the accesskey unless you first fold out the submenu which seems to be a regression from Firefox' 2 behaviour. We don't like to clutter up the interface with all kinds of links just to get the accesskeys working. In fact, having the possibility to assign accesskeys to certain links actually reduces the need for them to be directly visible.
Comment 2•16 years ago
|
||
Attached testcase works ok in firefox2, but fails on Firefox3. Tested on windows so OS should be changed to all.
Updated•16 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 3•16 years ago
|
||
This regressed on 28 Nov 2007 probably from Bug 143065.
Blocks: 143065
Keywords: regression,
testcase
OS: Linux → All
Hardware: PC → All
Version: unspecified → Trunk
Updated•16 years ago
|
Flags: blocking1.8.1.18?
Flags: blocking-firefox3.1?
Comment 4•16 years ago
|
||
Ria, thanks for the confirmation, but I've changed blocking 1.8.1.18 ? to blocking 1.9.0.3 ? since it's working on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.17) Gecko/20080829 Firefox/2.0.0.17 but not on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.2) Gecko/2008091620 Firefox/3.0.2. Please correct me if I'm wrong.
Flags: blocking1.8.1.18? → blocking1.9.0.3?
Comment 5•16 years ago
|
||
This isn't going to block a branch security release, but if it's taken on the next development release (3.1) we can look into approving the patch. It's possible that this is correct intended behavior (what does the spec say?). Moving bug to the correct component so the developers involved can have their say. Is this a CSS problem or an event problem?
Component: Keyboard Navigation → DOM: Events
Flags: wanted1.9.0.x?
Flags: blocking1.9.0.4?
Flags: blocking-firefox3.1?
Product: Firefox → Core
Event problem really. Should be relatively easy to fix.
Flags: blocking1.9.1?
Basically HTML4.01 doesn't say anything about the expected behaviour of accesskey on non-visible elements. In general it says that pressing an accesskey gives focus to the element it is defined for and executes it's default action. Now you could argue that non-visible elements are not focusable, however when you compare to common GUI design in normal (non-web) applications accelerator keys for for instance menu sub-options do work even when the menu isn't folded out and thus the sub-option isn't visible. For example, pressing CTRL+S does give me the 'save-as' dialog in Firefox even when the File-menu dropdown isn't visible.
Comment 8•16 years ago
|
||
Smaug, could you look into this? Not blocking the release on this, but we should fix this regression asap...
Assignee: nobody → Olli.Pettay
Flags: wanted1.9.1+
Flags: wanted-next+
Flags: blocking1.9.1?
Flags: blocking1.9.1-
Assignee | ||
Comment 9•16 years ago
|
||
Ok. I think I even know what this is about - hopefully a smallish fix to ESM.
Assignee | ||
Comment 10•16 years ago
|
||
Neil, I was thinking something like this. Special case XUL document and XUL elements. XUL just need to work a bit different way if we want to fix this bug. Now running rest of the tests...
Attachment #346868 -
Flags: review?(neil)
Assignee | ||
Comment 11•16 years ago
|
||
Comment on attachment 346868 [details] [diff] [review] possible fix Passes mochitest and browser-chrome, and those chrome tests which don't crash. (The crash is a spridermonkey bug, not related to this at all)
Assignee | ||
Comment 12•16 years ago
|
||
Attachment #346868 -
Attachment is obsolete: true
Attachment #346885 -
Flags: review?(neil)
Attachment #346868 -
Flags: review?(neil)
Updated•16 years ago
|
Attachment #346885 -
Flags: review?(neil) → review+
Comment 13•16 years ago
|
||
Comment on attachment 346885 [details] [diff] [review] early return It was hard to resist the temptation to microoptimise ;-)
Assignee | ||
Updated•16 years ago
|
Attachment #346885 -
Flags: superreview?(roc)
Shouldn't we be checking IsFocusable for all elements?
Assignee | ||
Comment 15•16 years ago
|
||
Well in nsEventStateManager::ExecuteAccessKey there is if (shouldActivate) content->PerformAccesskey(shouldActivate, aIsTrustedEvent); else if (frame && frame->IsFocusable()) ChangeFocusWith(content, eEventFocusedByKey); The patch brings us closer to the old (pre-bug-143065) behavior, when visibility nor focusability was a requirement for access keys to work.
Is there a reason to make non-focusable elements accesskey targets?
Comment 17•16 years ago
|
||
> Is there a reason to make non-focusable elements accesskey targets?
imo yes; it's part of standard interface design, like the way I can use accelerator keys in any application for actions that are in non-visible submenu's. We use it the same way on our website, and now those accesskeys stopped working for links that are in submenu's.
If this doesn't get fixed we'll need to mimic the old behaviour using javascript which is possible, but we'd rather not add this extra dependency for just one browser (we already have some javascripted workarounds for IE, but that's because of another issue - triggering an accesskey on a link in IE only focusses the link but does not activate it).
I don't see any reason not to make hidden/non-focusable elements targetable.
Assignee | ||
Comment 18•16 years ago
|
||
(In reply to comment #16) > Is there a reason to make non-focusable elements accesskey targets? That is what this bug is about.
Ah, when I read "hidden" I thought "visibility:hidden" not "display:none". Does HTML5 define how this should work? If not, I think we should bring it up there and get it defined properly. I'm not convinced that activating display:none content is always desirable.
Assignee | ||
Comment 20•16 years ago
|
||
HTML5 doesn't define this, but the testcase does work on Opera, Safari, FF2. Doesn't work on FF3/FF-trunk. IE7 seems to handle accesskeys in a bit different way - only focusing the link - so the testcase doesn't work there.
Then I think we should get the spec to define this, just to make sure people think it's the right thing.
Reporter | ||
Comment 22•16 years ago
|
||
Even if the outcome is that this should not be standard, it would be great for users to have the possibility to change that in about:chrome (or even the Preferences).
Assignee | ||
Comment 23•16 years ago
|
||
Roc, actually because comment #1 has a very valid use case (dhtml menus) for display:none accesskey, I'd really like to take the patch. I can sure wait a bit if someone on whatwg comes up with any counter arguments, but would be still better to get the patch in sooner than later.
Attachment #346885 -
Flags: superreview?(roc) → superreview+
Assignee | ||
Comment 24•16 years ago
|
||
Comment on attachment 346885 [details] [diff] [review] early return I won't land this on trunk, unless this gets also approval for 1.9.1. This brings back the accesskey behavior of FF2/Safari/Opera (well, in HTML, not in XUL)
Attachment #346885 -
Flags: approval1.9.1?
Comment 25•16 years ago
|
||
We might take this fix on 1.9.0 but not going to ever block or push for it so not "wanted".
Flags: wanted1.9.0.x?
Updated•15 years ago
|
Attachment #346885 -
Flags: approval1.9.1? → approval1.9.1+
Comment 26•15 years ago
|
||
Comment on attachment 346885 [details] [diff] [review] early return I think we should take this for 1.9.1 to restore compatibility with Firefox 2 etc.
Assignee | ||
Comment 27•15 years ago
|
||
Pushed to trunk http://hg.mozilla.org/mozilla-central/rev/101fb71a7066 Will wait few days before landing 1.9.1
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•15 years ago
|
Whiteboard: needs-1.9.1-landing
Assignee | ||
Comment 29•15 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a762223db9c1
Keywords: fixed1.9.1
Whiteboard: needs-1.9.1-landing
Comment 30•15 years ago
|
||
Any chance of this being fixed on 1.9.0.x?
You need to log in
before you can comment on or make changes to this bug.
Description
•