Diff Detail
- Repository
- rMOZILLACENTRAL mozilla-central
- Branch
- default
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
We are deprecating DTDs, so no new strings can be added to those files, and they need to be moved to Fluent. Bonus points if we move also the other string to Fluent ;-)
P.S. I assume this is a mentored bug?
I only know how to get something working with DTD atm and wanted a prototype. I definitely plan to use Fluent for the final version of this though. And yea it's a mentored bug (I think?). Thanks for response! I will try and get the other string into Fluent while I'm at it too.
| toolkit/components/pictureinpicture/PictureInPicture.jsm | ||
|---|---|---|
| 684 | This is making me wonder if we should evaluate these horizontal toggle positions as constants. Something like: if (PictureInPicture.pipTogglePosition == TOGGLE_POSITION_RIGHT) {
// ....
}We could create, export, and then import these constants from the https://searchfox.org/mozilla-central/source/toolkit/components/pictureinpicture/PictureInPictureControls.jsm module | |
| 845 | Should default to "right" | |
I kind of feel like we should make this per-site instead of just global (although perhaps still have the global as a default?), so that either we or the user can move it to the appropriate spot for each site, similar to how the current per-site policy for setting the Y position of the toggle works. Does that make sense?
| browser/locales/en-US/chrome/browser/browser.dtd | ||
|---|---|---|
| 29 | Chatted briefly about this with @whjones526 on Matrix, we might consider changing this string to something more explicit. Perhaps something like: "Left-align Picture-in-Picture toggle" | |
| browser/locales/en-US/chrome/browser/browser.dtd | ||
|---|---|---|
| 29 | "Move Picture-in-Picture toggle to [left,right] side"? "Left-align" feels slightly jargon-y. | |
Are we suggesting we expose changing the site-specific toggle policies to the user here? I think that could work and would honestly open a lot more options for user customization of PiP down the road. If we decided to go down this route, I can see this becoming more involved.
I'm not entirely sure what the best way to manage all these specific toggle overrides would be. It seems like the current per-site policy is hardcoded at https://searchfox.org/mozilla-central/rev/6a023272d590409c80458d373986e379b3ad86f4/browser/extensions/webcompat/data/picture_in_picture_overrides.js and I suspect we can change these overrides via https://searchfox.org/mozilla-central/rev/6a023272d590409c80458d373986e379b3ad86f4/browser/extensions/webcompat/experiment-apis/pictureInPicture.js#42-52. But then we need some way to persist these overrides, which, I think might be more complicated. So far I'm only aware of storing these as either prefs, which doesn't seem too ideal, or creating some local DB (kind of like bookmarks).
| browser/locales/en-US/chrome/browser/browser.dtd | ||
|---|---|---|
| 29 | Good point. I think that suggestion is a good compromise | |
| browser/locales/en-US/chrome/browser/browser.dtd | ||
|---|---|---|
| 29 | Is the indicator normally on the right for RTL builds? | |
| browser/locales/en-US/chrome/browser/browser.dtd | ||
|---|---|---|
| 29 | It's not. If we end up with "left"/"right", please make sure to write that as a comment so RTL localizers would know to translate as the opposite, like in https://searchfox.org/mozilla-central/rev/6806b58cd25a88aad78aa3dda9c3f3aa75278d78/browser/locales/en-US/browser/tabContextMenu.ftl#17-23 | |
| browser/locales/en-US/chrome/browser/browser.dtd | ||
|---|---|---|
| 29 | Please disregard my previous comment, I thought this was about the PIP itself, and not the toggle. Yes, the toggle is on the right for RTL builds. | |
That is what I had in mind, yeah. Although now, I do think we should probably ask for product and/or UX feedback here. I'm not fully comfortable making the call about how best to handle this from a UX perspective, and I hadn't considered the extra effort in building the data store for the site-specific settings (I agree that using prefs should not be the first choice), which should also be taken into account.
My apologies for taking a while to get back to this, by the way.