Closed Bug 597218 Opened 9 years ago Closed 9 years ago
It shouldn't be possible for app tabs to be hidden
Working on another bug (bug 595943), I discovered that it's possible for xul:tabs to have both .pinned and .hidden true. They're still visible in this state (evidently pinned takes precedence), but it's possible they behave badly in other ways. So I guess there's really two issues here: Panorama should never put tabs in that state, and also it probably shouldn't even be possible to do so. This bug is for the latter; I'll file another bug for the former.
Bug 597220 is for the Panorama-specific aspect.
hideTab() actually refuses hiding pinned tabs, doesn't it?
Presumably when we fix bug 597220, we'll know how we got into that state.
(In reply to comment #2) > hideTab() actually refuses hiding pinned tabs, doesn't it? It does, but pinTab doesn't check to see if the tab is hidden. I propose pinning a tab should unhide it.
This should do it, I think; just needs a test.
Attachment #479543 - Flags: feedback?(dao)
Now with test. Of course people can still set pinned and hidden directly (without calling pinTab and hideTab) and get themselves in this state. Would it make sense to change those properties into getters/setters?
It may make sense to make 'hidden' read-only, 'pinned' already is.
(In reply to comment #7) > It may make sense to make 'hidden' read-only, 'pinned' already is. Shall we do that in this bug? Looks like hidden isn't an official property of tabbrowser-tab. Would making it read-only be just adding: <property name="hidden" readonly="true"> <getter> return this.getAttribute("hidden") == "true"; </getter> </property> ... to tabbrowser-tab's implementation block, or is there more that would need to be done?
(In reply to comment #8) > (In reply to comment #7) > > It may make sense to make 'hidden' read-only, 'pinned' already is. > > Shall we do that in this bug? Your choice... > ... to tabbrowser-tab's implementation block, or is there more that would need > to be done? I think that should work. tabbrowser.xml and nsSessionStore.js currently set the hidden property and would need to stop doing that.
This patch should make it read-only, and fix all the places it was being set directly. Do we need to update documentation to indicate that it's read-only? If so, how do we flag that?
(In reply to comment #10) > Do we need to update documentation to indicate that it's read-only? If so, how > do we flag that? You would add the dev-doc-needed keyword. However, 'hidden' is a general-purpose property which people weren't supposed to use for tabs in the first place, so I'm not sure there's something to update.
http://hg.mozilla.org/mozilla-central/rev/75b7652a6e2b Adding dev-doc-needed; the docs folks can determine whether there's anything to be done on their end.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Nope, nothing to do for docs; docs never suggested tabs could be hidden.
You need to log in before you can comment on or make changes to this bug.