Closed
Bug 810260
Opened 11 years ago
Closed 11 years ago
xul:deck hidden pages shouldn't be offscreen
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla19
People
(Reporter: surkov, Assigned: surkov)
References
(Blocks 1 open bug)
Details
(Keywords: access, regression)
Attachments
(1 file)
4.45 KB,
patch
|
tbsaunde
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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•11 years ago
|
Keywords: regression
Comment 1•11 years ago
|
||
Isn't that regression related to bug 750612 ?
Assignee | ||
Comment 2•11 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•11 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?
Comment 4•11 years ago
|
||
(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•11 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.
Comment 6•11 years ago
|
||
(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•11 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).
Comment 8•11 years ago
|
||
(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•11 years ago
|
||
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #681936 -
Flags: review?(trev.saunders)
Updated•11 years ago
|
Attachment #681936 -
Flags: review?(trev.saunders) → review+
Comment 10•11 years ago
|
||
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•11 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?
Comment 12•11 years ago
|
||
(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•11 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/6ef7a00233aa
Comment 14•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6ef7a00233aa
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Assignee | ||
Comment 15•11 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•11 years ago
|
Flags: in-testsuite+
Comment 16•11 years ago
|
||
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•11 years ago
|
Keywords: checkin-needed
Comment 17•11 years ago
|
||
Pushed: https://hg.mozilla.org/releases/mozilla-aurora/rev/bdd31f2f2b08
You need to log in
before you can comment on or make changes to this bug.
Description
•