Closed Bug 1195150 Opened 4 years ago Closed 4 years ago

Persist the tab muted state in the session store

Categories

(Firefox :: Session Restore, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 43
Tracking Status
firefox42 --- verified
firefox43 --- verified

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(1 file)

The muted state is state that belong to the whole tab, and it would be
nice if it was integrated with undo close tab and session restore.
Assignee: nobody → ehsan
Comment on attachment 8648531 [details] [diff] [review]
Persist the tab muted state in the session store

Review of attachment 8648531 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not sure if we want to put "muted" in the attribute set here, as I would have expected to see other attribute names present if this was the desired usage here. I would redirect to Tim but he's on PTO until August 31st, but Bill has reviewed a lot of Tim's patches in this file so maybe he can give some guidance here.
Attachment #8648531 - Flags: review?(jaws) → review?(wmccloskey)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #2)
> I'm not sure if we want to put "muted" in the attribute set here, as I would
> have expected to see other attribute names present if this was the desired
> usage here. I would redirect to Tim but he's on PTO until August 31st, but
> Bill has reviewed a lot of Tim's patches in this file so maybe he can give
> some guidance here.

FWIW right now we use the tab attribute persistence functionality in TabAttributes.jsm only for tests, and by default we don't remember or restore any tab attributes.  Things such as the tab's pinned-ness are persisted in TabState.jsm (for example, see <http://mxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/TabState.jsm#157>)

I can do something similar to that here too, but it seems easier to just piggy back on the generic attribute persistence code.
Comment on attachment 8648531 [details] [diff] [review]
Persist the tab muted state in the session store

Review of attachment 8648531 [details] [diff] [review]:
-----------------------------------------------------------------

I think this is fine. This API is mostly designed for add-ons as far as I understand, but there's no reason we can't use it. The only danger is that some add-on is already using the name "muted". I don't see anything like that in MXR though.
Attachment #8648531 - Flags: review?(wmccloskey) → review+
Comment on attachment 8648531 [details] [diff] [review]
Persist the tab muted state in the session store

Approval Request Comment
[Feature/regressing bug #]: Tab audio indicators
[User impact if declined]: See comment 0.  This is a small polish patch.
[Describe test coverage new/current, TreeHerder]: Has a test.
[Risks and why]: Extremely low risk.
[String/UUID change made/needed]: None.
Attachment #8648531 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/7fbaf7e0566d
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Comment on attachment 8648531 [details] [diff] [review]
Persist the tab muted state in the session store

Polish this new feature, let's take it!
Attachment #8648531 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed using latest Nightly 43.0a1 (buildID: 20150825030212) and latest Aurora 42.0a2 (buildID: 20150825004008).
Status: RESOLVED → VERIFIED
Depends on: 1211314
You need to log in before you can comment on or make changes to this bug.