Unbox icon indicating copy to clipboard operation
Unbox copied to clipboard

feat: WIP support for user style overrides.

Open KerfuffleV2 opened this issue 2 years ago • 5 comments

Proof of concept for allowing user style overrides.

Adds more CSP policies to further restrict the stash-list page privileges. Unfortunately I don't see a way to get around style-src 'unsafe-inline' part. Ideally a nonce or something would be used instead, but the stash-list.html page is static. The other policies should prevent anything like loading/importing remote resources.

The preferences part is really rough, although it is usable. Obviously it would need actual warnings for the dangers rather than those placeholders.

These styles only affect the stash-list page, so it's not possible to do something like make the options page unusable. Even something like html { display: none !important; } removes all content from the stash-list but can easily be reverted from the options.

KerfuffleV2 avatar Mar 14 '22 10:03 KerfuffleV2

Status

  • [ ] Verify safety.
  • [ ] Verify permission for feature with Mozilla.
  • [ ] Show user appropriate disclaimers/warnings/opt in prompts before allowing access to feature.
  • [ ] Improve preference interface.

Anything else that should be added here?

Safety Verification

  • [x] Prevents access to remote resources in CSS.
  • [ ] Prevents access to remote resources from embedded SVG. (not tested)
  • [x] Cannot affect preferences or pages other than the stash-list.
  • [x] Cannot escape enclosing <style> tag.

Any other security-related tests to add?

KerfuffleV2 avatar Mar 15 '22 11:03 KerfuffleV2

Anything else that should be added here?

I think that covers it!

On the UI front, what do you think of putting it in a dialog or maybe even in a separate page (double secret advanced settings?) and linking/adding a button on the main settings page?

Also, what do you think of syncing the CSS but limiting the size to something that would fit in synced storage (like 10KB)? Would avoid the need to manually sync between browsers. Just as a point of comparison, the entire set of Less files under styles is ~80KB.

josh-berry avatar Mar 20 '22 21:03 josh-berry

I think that covers it!

Great! I've looked around and it doesn't seem like there's a clear way to communicate with Mozilla and ask them if doing something is okay. I haven't had a chance yet, but I plan to look into the forums/Matrix channel they link to. I'm not sure if that's an avenue that will get an official response, but I will at least try.

If I can't get an official response would showing that other popular tab-handling addons already have this feature without it (apparently) causing issues work? For example:

Tree Style Tab has a section on CSS customizations: https://github.com/piroor/treestyletab/wiki/Code-snippets-for-custom-style-rules#for-version-20-and-later (you can see from the screenshot at the start of that section there aren't even really warnings/disclaimers and it's visible without "expert options" being unlocked too.)

Sidebery has this feature as well: https://github.com/mbnuqw/sidebery/wiki/Sidebery-Styles-Snippets — it's just a text box you can dump CSS into, there are no warnings or anything at all just a prompt to "write custom CSS here".

Those are the two I can recall off the top of my head, but I know I've seen other addons with that feature as well.

On the UI front, what do you think of putting it in a dialog or maybe even in a separate page (double secret advanced settings?) and linking/adding a button on the main settings page?

Sure, whichever you'd prefer! The only thing I'd probably suggest against is a dialog since part of iterating on making changes is observing how they look under different conditions (like expanding/closing folders, triggering a tab to load, etc). A dialog could make interacting with the rest of the browser difficult, at least if it was handled like a modal.

Also, what do you think of syncing the CSS but limiting the size to something that would fit in synced storage (like 10KB)?

I think it's reasonable to disable syncing at a a certain point, but I'd prefer not to have a hard limit that would affect people who aren't even using sync. I personally don't use it at all.

Just for reference, the changes I posted over in #232 come to over 3k and they're generally pretty simple. I think running into a 10k limit would be pretty easy if someone trying to make a real theme, especially if they were doing stuff like including icons in data: links.

Maybe have it as a non-synced preference but include something like a Sync button/checkbox/whatever that could be disabled with a warning if the content was too long?

KerfuffleV2 avatar Mar 22 '22 08:03 KerfuffleV2

Great! I've looked around and it doesn't seem like there's a clear way to communicate with Mozilla and ask them if doing something is okay. I haven't had a chance yet, but I plan to look into the forums/Matrix channel they link to. I'm not sure if that's an avenue that will get an official response, but I will at least try.

Since there seems to be precedent elsewhere (even amongst Recommended extensions), we can probably ask forgiveness instead of permission. ;) I can make a point of highlighting it in the reviewer notes when I submit.

I think as long as we take care to follow the "no surprises" rule and people are warned about the risks, it would likely be accepted.

On the UI front, what do you think of putting it in a dialog or maybe even in a separate page (double secret advanced settings?) and linking/adding a button on the main settings page?

Sure, whichever you'd prefer! The only thing I'd probably suggest against is a dialog since part of iterating on making changes is observing how they look under different conditions (like expanding/closing folders, triggering a tab to load, etc). A dialog could make interacting with the rest of the browser difficult, at least if it was handled like a modal.

The modality of the dialog would have to be limited to the options page (you could still interact with other windows/tabs/the sidebar), but I think I would lean towards a separate page anyway for a few reasons:

  1. Gives more visual room to edit the CSS
  2. Gives another place to add debugging/diagnostic facilities in the future if need be
  3. It's easier to show a warning

Also, what do you think of syncing the CSS but limiting the size to something that would fit in synced storage (like 10KB)?

... Maybe have it as a non-synced preference but include something like a Sync button/checkbox/whatever that could be disabled with a warning if the content was too long?

Seems like a good approach to me. Maybe it could even be automatic below the size cutoff (with an appropriate warning displayed above the cutoff). Then you get the best of both worlds--automatic syncing where possible, and a clear warning when not.

josh-berry avatar Mar 27 '22 22:03 josh-berry

I think as long as we take care to follow the "no surprises" rule and people are warned about the risks, it would likely be accepted.

I agree with that.

The rest also sounds reasonable to me. I'll look into actually trying to implement it when I get a chance, but I'm not really sure when that will be. Hopefully some time this month.

KerfuffleV2 avatar Apr 05 '22 16:04 KerfuffleV2