xul:deck hidden pages shouldn't be offscreen

RESOLVED FIXED in Firefox 18

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

(Blocks: 1 bug, {access, regression})

unspecified
mozilla19
access, regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox18 fixed, firefox19 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Jamie gave me an example on irc. Help -> About dialog has xul:deck containing update controls (they are visible depending on firefox update state). Those controls shouldn't be really marked offscreen but rather invisible.

Suggestion was:
So tabpanels should do offscreen state
deck should do invisible state

alternatively we could ignore to create accessibles for deck and for hidden panels.

Thoughts?
(Assignee)

Updated

5 years ago
Keywords: regression
Isn't that regression related to bug 750612 ?
(Assignee)

Comment 2

5 years ago
(In reply to Hub Figuiere [:hub] from comment #1)
> Isn't that regression related to bug 750612 ?

I believe it was bug 686821.
Blocks: 686821
(Assignee)

Comment 3

5 years ago
Trev, your opinion. (We should be fast to fix this in Firefox 19 and back port to Firefox 18 where we regressed).
Flags: needinfo?
(In reply to alexander :surkov from comment #0)
> Jamie gave me an example on irc. Help -> About dialog has xul:deck
> containing update controls (they are visible depending on firefox update
> state). Those controls shouldn't be really marked offscreen but rather
> invisible.

so this is a kind of funny use of set of tabs where which one is visible is controled by program not user  right?

> Suggestion was:
> So tabpanels should do offscreen state
> deck should do invisible state

why do you think tab pannels are more like for the user controlled case, and deck for the program controlled one?

> alternatively we could ignore to create accessibles for deck and for hidden
> panels.

I thing I vaguely think invisible accessible is safer than no  accessible.
Flags: needinfo?
(Assignee)

Comment 5

5 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #4)
> (In reply to alexander :surkov from comment #0)
> > Jamie gave me an example on irc. Help -> About dialog has xul:deck
> > containing update controls (they are visible depending on firefox update
> > state). Those controls shouldn't be really marked offscreen but rather
> > invisible.
> 
> so this is a kind of funny use of set of tabs where which one is visible is
> controled by program not user  right?

those aren't really tabs, I miss the question though

> > Suggestion was:
> > So tabpanels should do offscreen state
> > deck should do invisible state
> 
> why do you think tab pannels are more like for the user controlled case, and
> deck for the program controlled one?

it seems all we can do is try to figure out empirical regularities since deck frame is common engine for both cases.

> > alternatively we could ignore to create accessibles for deck and for hidden
> > panels.
> 
> I thing I vaguely think invisible accessible is safer than no  accessible.

safer in terms AT can workaround specific cases I guess, that's not what we'd want it seems.
(In reply to alexander :surkov from comment #5)
> (In reply to Trevor Saunders (:tbsaunde) from comment #4)
> > (In reply to alexander :surkov from comment #0)
> > > Jamie gave me an example on irc. Help -> About dialog has xul:deck
> > > containing update controls (they are visible depending on firefox update
> > > state). Those controls shouldn't be really marked offscreen but rather
> > > invisible.
> > 
> > so this is a kind of funny use of set of tabs where which one is visible is
> > controled by program not user  right?
> 
> those aren't really tabs, I miss the question though

I'm not sure there was a question.  What would you say they are if not tabs?

> > > Suggestion was:
> > > So tabpanels should do offscreen state
> > > deck should do invisible state
> > 
> > why do you think tab pannels are more like for the user controlled case, and
> > deck for the program controlled one?
> 
> it seems all we can do is try to figure out empirical regularities since
> deck frame is common engine for both cases.

That sounds like it will be broken for some cases no matter what we do, but I'm not sure exactly what you suggest.  Is there a reason we can't add attributes to xul to distinguish these cases?  Is there a reason the not visible tabs can't be made display: none / hidden?

> > > alternatively we could ignore to create accessibles for deck and for hidden
> > > panels.
> > 
> > I thing I vaguely think invisible accessible is safer than no  accessible.
> 
> safer in terms AT can workaround specific cases I guess, that's not what
> we'd want it seems.

true, that's atleast part of the reason, it also just feels more uniform or something, but I don't feel terribly strongly about this.
(Assignee)

Comment 7

5 years ago
Currently we have two cases:

tabpanels is used for tabs
deck is used for that About dialog

My suggestion was: expose offscreen state for tabpanels elements, expose invisible state for deck elements.

extra attribute on deck might be a good idea if we had a case of deck element used as tabpanels (where the user can manipulate the index).
(In reply to alexander :surkov from comment #7)
> Currently we have two cases:
> 
> tabpanels is used for tabs
> deck is used for that About dialog
> 
> My suggestion was: expose offscreen state for tabpanels elements, expose
> invisible state for deck elements.

that seems fine.

> extra attribute on deck might be a good idea if we had a case of deck
> element used as tabpanels (where the user can manipulate the index).

ok, I guess I'm fine to worry about this when and if it becomes an issue.
(Assignee)

Comment 9

5 years ago
Created attachment 681936 [details] [diff] [review]
patch
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #681936 - Flags: review?(trev.saunders)
Attachment #681936 - Flags: review?(trev.saunders) → review+
Comment on attachment 681936 [details] [diff] [review]
patch

>+    if (deckFrame && deckFrame->GetSelectedBox() != curFrame) {
>+      if (deckFrame->GetContent()->NodeInfo()->Equals(nsGkAtoms::tabpanels,
>+                                                      kNameSpaceID_XUL))

maybe we should add IsXUL(tag) like IsHTML(tag) etc?
(Assignee)

Comment 11

5 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #10)
> Comment on attachment 681936 [details] [diff] [review]
> patch
> 
> >+    if (deckFrame && deckFrame->GetSelectedBox() != curFrame) {
> >+      if (deckFrame->GetContent()->NodeInfo()->Equals(nsGkAtoms::tabpanels,
> >+                                                      kNameSpaceID_XUL))
> 
> maybe we should add IsXUL(tag) like IsHTML(tag) etc?

IsXUL() && Tag() == nsGKAtoms::tabpanels isn't it roughly the same?
(In reply to alexander :surkov from comment #11)
> (In reply to Trevor Saunders (:tbsaunde) from comment #10)
> > Comment on attachment 681936 [details] [diff] [review]
> > patch
> > 
> > >+    if (deckFrame && deckFrame->GetSelectedBox() != curFrame) {
> > >+      if (deckFrame->GetContent()->NodeInfo()->Equals(nsGkAtoms::tabpanels,
> > >+                                                      kNameSpaceID_XUL))
> > 
> > maybe we should add IsXUL(tag) like IsHTML(tag) etc?
> 
> IsXUL() && Tag() == nsGKAtoms::tabpanels isn't it roughly the same?

yeah, just shorter.  besides its kind of inconsistant we can do content->IsHTML(nsGkAtoms::div) or content->IsSVG(nsGkAtoms::Description) but not content->IsXUL(nsGkAtoms::Tabpannels)
(Assignee)

Comment 13

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/6ef7a00233aa
https://hg.mozilla.org/mozilla-central/rev/6ef7a00233aa
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
(Assignee)

Comment 15

5 years ago
Comment on attachment 681936 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 686821
User impact if declined: may confuse screen readers/users since it makes elements that are invisible for sighted users visible for screen reader users.
Testing completed (on m-c, etc.): m-c, behavior is covered by testsuite
Risk to taking this patch (and alternatives if risky): low risk (roll back to previous behavior)
String or UUID changes made by this patch: no
Attachment #681936 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

5 years ago
Flags: in-testsuite+
Comment on attachment 681936 [details] [diff] [review]
patch

[Triage Comment]
Low risk fix for a FF18 a11y regression - approving for FF18.

If this is landed on mozilla-aurora before ~11AM PT tomorrow, this will make the merge from Aurora 18 to Beta 18. If landed 11AM-5PM PT tomorrow on mozilla-beta, it will make the first FF18 Beta. If landed after that, it will end up in the second FF18 beta.
Attachment #681936 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
Pushed: https://hg.mozilla.org/releases/mozilla-aurora/rev/bdd31f2f2b08
status-firefox18: --- → fixed
status-firefox19: --- → fixed
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.