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)

defect
Not set
minor

Tracking

()

RESOLVED FIXED

People

(Reporter: jhsoby, Assigned: smaug)

References

Details

(Keywords: fixed1.9.1, regression, testcase)

Attachments

(2 files, 1 obsolete file)

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.
Attached file testcase
Attached testcase works ok in firefox2, but fails on Firefox3. Tested on windows so OS should be changed to all.
Status: UNCONFIRMED → NEW
Ever confirmed: true
This regressed on 28 Nov 2007 probably from Bug 143065.
Blocks: 143065
Keywords: regression, testcase
OS: Linux → All
Hardware: PC → All
Version: unspecified → Trunk
Flags: blocking1.8.1.18?
Flags: blocking-firefox3.1?
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?
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.
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-
Ok. I think I even know what this is about - hopefully a smallish fix to ESM.
Attached patch possible fix (obsolete) — Splinter Review
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)
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)
Attached patch early returnSplinter Review
Attachment #346868 - Attachment is obsolete: true
Attachment #346885 - Flags: review?(neil)
Attachment #346868 - Flags: review?(neil)
Attachment #346885 - Flags: review?(neil) → review+
Comment on attachment 346885 [details] [diff] [review]
early return

It was hard to resist the temptation to microoptimise ;-)
Attachment #346885 - Flags: superreview?(roc)
Shouldn't we be checking IsFocusable for all elements?
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?
> 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.
(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.
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.
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).
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+
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?
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?
Attachment #346885 - Flags: approval1.9.1? → approval1.9.1+
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.
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
Whiteboard: needs-1.9.1-landing
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.