josh-berry / tab-stash

Firefox extension to save and restore tabs as bookmarks. Clear your tabs, clear your mind.
https://josh-berry.github.io/tab-stash/
Mozilla Public License 2.0
774 stars 44 forks source link

Proposing some low priority quality of life features. #232

Open KerfuffleV2 opened 2 years ago

KerfuffleV2 commented 2 years ago

This is just asking if these are features you'd generally accept PRs for, not asking you to implement them yourself. I figured this was better than creating a bunch of issues, but let me know if you'd like a different approach.

josh-berry commented 2 years ago
  1. Add an option to allow more aggressive deduplication when stashing. More specifically, attempting to stash an open tab that exists in the stash would simply close the tab.

Yes, although I want to be careful of being too aggressive--Tab Stash actually used to have a de-duplication feature a long time ago, but I had to remove it because it was causing issues with Sync. So I have a few constraints:

  1. Many tab addons have an indicator for new tabs that haven't been active yet. This makes it easier to find tabs that have been opened in the background. This could be behind an option that defaulted to off.

  2. Some visual indication for open tabs that have already been stashed. Probably also add the stash older to the tab tooltip. Could be behind an option that defaulted to off.

Yes to both of these, though we'd have to make sure they play nicely together and the UI doesn't get too busy.

  1. Add more keyboard bind settings for convenience functions like focusing the search box if the sidebar is open. They could default to not being bound at all.

Yes. I'd also like to get some better keyboard-based navigation in place. See also #131.

KerfuffleV2 commented 2 years ago

@josh-berry

Yes, although I want to be careful of being too aggressive

Maybe I'm confused, but it doesn't sound like we're talking about the same thing here. Your response sounds like it's referring to deduplicating the existing stash.

What I was talking about is just refusing to create (new) duplicates when performing stash operations. That way the stash hotkey, for example, could be used as a general purpose "close this tab I'd like to be able to come back to" — and if it already existed in the stash, it would just close. Otherwise it would get stashed.

Also, I was definitely talking about making it optional. I'd go with making anything that changes existing behavior opt-in unless it's very uncontroversial.

Yes to both of these, though we'd have to make sure they play nicely together and the UI doesn't get too busy.

Agree. I think both of those should use indicators that are pretty low-key.

Regarding the tab stashed indicator: After some of my other PRs get merged (mainly the group stats one, because it touches this stuff) I want to look at a way to aggregate and make that sort of information available efficiently. Right now it seems like pulling that data could have a noticeable performance impact if there are a lot of bookmarks/tabs.

I'd also like to get some better keyboard-based navigation in place. See also #131.

Ah, good. I'll edit that in. The big one I'd be looking for personally is an "open (but don't toggle) the sidebar and focus search" binding.


On a slightly different topic: Some extensions give the use the ability to plug in CSS to style the extension. How do you feel about allowing something like that? Since CSS could be arbitrary size it would probably have to be an unsynced setting. It would allow people to add their own tweaks to the interface (and possibly share them via wiki?)

Just for example, I'd want to make the interface more compact and get rid of the huge ... for truncated titles. I like the approach Sideberry uses where it just shades the text as it is clipped:

Tab Stash Sidebery
image image

Another thought: How committed are you to having left click to edit folder names? Right now, you can click anywhere on the label to edit it, even though edit is probably something people don't do very often. However toggling expansion requires clicking a small button, and is a common function.

I actually am using this change in my personal version (which I haven't submitted a PR for yet, although I could!): https://github.com/KerfuffleV2/tab-stash/tree/feat-middleclick-toggles-folder-expansion

That just allows middle click on a folder to toggle it, which I think is an improvement but if it were up to me I'd change left click to toggle the folder and just add a button for editing it (which could be in the same place as the toggle one, so it wouldn't add to UI clutter or anything.)

josh-berry commented 2 years ago

@josh-berry

Yes, although I want to be careful of being too aggressive

Maybe I'm confused, but it doesn't sound like we're talking about the same thing here. Your response sounds like it's referring to deduplicating the existing stash.

What I was talking about is just refusing to create (new) duplicates when performing stash operations. That way the stash hotkey, for example, could be used as a general purpose "close this tab I'd like to be able to come back to" — and if it already existed in the stash, it would just close. Otherwise it would get stashed.

Also, I was definitely talking about making it optional. I'd go with making anything that changes existing behavior opt-in unless it's very uncontroversial.

Got it, I think I may have misunderstood what you were originally proposing. This sounds fine to me.

Regarding the tab stashed indicator: After some of my other PRs get merged (mainly the group stats one, because it touches this stuff) I want to look at a way to aggregate and make that sort of information available efficiently. Right now it seems like pulling that data could have a noticeable performance impact if there are a lot of bookmarks/tabs.

👍

On a slightly different topic: Some extensions give the use the ability to plug in CSS to style the extension. How do you feel about allowing something like that? Since CSS could be arbitrary size it would probably have to be an unsynced setting. It would allow people to add their own tweaks to the interface (and possibly share them via wiki?)

I would be very concerned about security issues. I remember reading somewhere (but can't find it now) that if you have the ability to inject CSS into a page, it's also possible to execute arbitrary code from that page. (And since Tab Stash is a privileged extension context and asks for a lot of permissions, it might be possible to do more damage than if it were allowed on a normal page.)

That said, my info could be out of date; I'm actually surprised Mozilla allowed Sidebery to do it.

I'd be open to more limited editing capabilities (e.g. the ability to change colors, padding, font sizes, etc.). I suspect that would be a much heavier lift in terms of development effort though.

Just for example, I'd want to make the interface more compact and get rid of the huge ... for truncated titles. I like the approach Sideberry uses where it just shades the text as it is clipped: ...

I'm happy to take improvements to the "Compact" style if that helps.

Another thought: How committed are you to having left click to edit folder names? Right now, you can click anywhere on the label to edit it, even though edit is probably something people don't do very often. However toggling expansion requires clicking a small button, and is a common function.

I actually am using this change in my personal version (which I haven't submitted a PR for yet, although I could!): https://github.com/KerfuffleV2/tab-stash/tree/feat-middleclick-toggles-folder-expansion

That just allows middle click on a folder to toggle it, which I think is an improvement but if it were up to me I'd change left click to toggle the folder and just add a button for editing it (which could be in the same place as the toggle one, so it wouldn't add to UI clutter or anything.)

I'd be open to allowing middle-click to collapse/expand a folder, but I'd like to keep left-click for editing; I think it's more intuitive to just click on the title itself to edit it. (My to-do list app has a similar flow where to edit a task I have to click the little "Edit" icon instead of just clicking the task title, and it drives me nuts.)

KerfuffleV2 commented 2 years ago

Got it, I think I may have misunderstood what you were originally proposing. This sounds fine to me.

:+1:

I would be very concerned about security issues. I remember reading somewhere (but can't find it now) that if you have the ability to inject CSS into a page, it's also possible to execute arbitrary code from that page.

Well, in this case it would be the user choosing exactly what to add and presumably they wouldn't be exploiting themselves. :)

I don't think this is something that would require any changes to Tab Stash's existing permissions though, or even "injecting" CSS into arbitrary pages. What I'm thinking of is basically like having the Vue template generate a <style>USER CSS</style> block. What we'd probably actually want to do is have it as a resource under the extension though so it could just be imported as a file. That way something like a missing close quote or bracket couldn't screw up the rest of the page.

By the way, did you see my post over here? https://github.com/josh-berry/tab-stash/issues/87#issuecomment-1063250404

It's already possible to do this, it's just a lot less convenient and of course messing with those files amplifies any possible security consideration since they can affect all browser/extension pages and inject CSS content.

I'm happy to take improvements to the "Compact" style if that helps.

How about adding a new "Ultracompact" style or something so people who like the current compact theme don't have to adjust? Maybe a darker theme too. :) I'm actually using that userContent.css approach to make stuff smaller and the background darker for better contrast.

This is what my current changes look like, if you're curious: image

I'd be open to allowing middle-click to collapse/expand a folder, but I'd like to keep left-click for editing

Fair enough, I can live with that. I submitted #238 which is a super-tiny change. Perhaps it should also add a new line to folder/tab list tooltips that says (Middle click to toggle expansion) or something like that?

I think it's more intuitive to just click on the title itself to edit it.

I don't necessarily disagree, for me it's more about making common operations convenient. At least for me, I very rarely change folder titles but frequently expand/collapse them

Mikhail22 commented 2 years ago

Inserting my opinions here regading the UI:

Agree with @KerfuffleV2 on:

More opinions on UI design:

And of course it would be great to have user style customizations.

josh-berry commented 2 years ago

@KerfuffleV2:

I would be very concerned about security issues. I remember reading somewhere (but can't find it now) that if you have the ability to inject CSS into a page, it's also possible to execute arbitrary code from that page.

Well, in this case it would be the user choosing exactly what to add and presumably they wouldn't be exploiting themselves. :)

You are assuming that users actually understand what they are putting in that box, rather than randomly copy/pasting in something they found in a forum post somewhere. ;)

I don't think this is something that would require any changes to Tab Stash's existing permissions though, or even "injecting" CSS into arbitrary pages. What I'm thinking of is basically like having the Vue template generate a <style>USER CSS</style> block. What we'd probably actually want to do is have it as a resource under the extension though so it could just be imported as a file. That way something like a missing close quote or bracket couldn't screw up the rest of the page.

It's not an issue of feasibility/difficulty so much as a "this probably violates Mozilla's add-on policies and/or opens a security hole" issue.

In fact, here's an example where it DID open a security hole in Slack: https://hackerone.com/reports/679969

By the way, did you see my post over here? #87 (comment)

It's already possible to do this, it's just a lot less convenient and of course messing with those files amplifies any possible security consideration since they can affect all browser/extension pages and inject CSS content.

I did, and intentionally haven't commented because while I'm generally supportive of people modding and customizing (Tab Stash is open source, after all!), I can't provide user support for such things. (Even apart from potential security issues, applying arbitrary CSS opens all kinds of avenues for breaking things in subtle ways, and there is no guarantee that a mod will work from version to version.)

I'm happy to take improvements to the "Compact" style if that helps.

How about adding a new "Ultracompact" style or something so people who like the current compact theme don't have to adjust? Maybe a darker theme too. :) I'm actually using that userContent.css approach to make stuff smaller and the background darker for better contrast.

I think tweaking the current "Compact" style in small ways probably won't cause any consternation. Current users might even appreciate some of these tweaks.

I'd be open to allowing middle-click to collapse/expand a folder, but I'd like to keep left-click for editing

Fair enough, I can live with that. I submitted #238 which is a super-tiny change. Perhaps it should also add a new line to folder/tab list tooltips that says (Middle click to toggle expansion) or something like that?

Yeah I think that would be good, to help with discoverability.


@Mikhail22:

  • Collapse/expand by clicking on the group header would be better than rename, because collapsing/expanding is frequently used (Rename button could be added for renaming)

Regarding the collapse/expand vs. rename behavior, and in general, it's more important to me to avoid user confusion (and keep things intuitive even for non-power users) than it is to micro-optimize for efficiency. I want to strike a balance between power-user and normal-user comforts--Tab Stash should be approachable on first use AND frictionless thereafter. Sometimes those things are in conflict, and in that case, approachability usually wins.

That said, I'm open to adding more options for customization in the future--everyone's working style is different and I ALSO want to be sensitive to that.

Regarding your other points, I have been thinking about how to refresh the UI (which is overdue, since the current design actually predates Firefox's UI refresh), and that includes things like the separator line, icon designs, fonts, spacing, etc. However:

  • I'd avoid the bold font for the active tab, instead use cell bg highlighting

This unfortunately won't work because it conflicts with selection and hover effects, and I also think the bold font helps to make it easier to find the active tab in the list.

  • is the button to 'open all .. and delete the group' really needed? There are 'open all..' and 'delete the group' buttons already.

Yes. Even though it seems like a very small thing, this is actually one of the differentiating features of Tab Stash, because it makes it very easy to switch between groups of tabs in as few clicks as possible, which is something users have told me is very important to them (and that I myself have found very useful especially for temporary/unnamed groups).

For example: https://addons.mozilla.org/en-US/firefox/addon/tab-stash/reviews/1695290/

KerfuffleV2 commented 2 years ago

@josh-berry

You are assuming that users actually understand what they are putting in that box

It's more that I only believe in going so far in protecting people from themselves. If they ignore a warning about not pasting in stuff then don't understand from untrusted sources then it's their own responsibility.

It could also only apply to the stash list page so it wouldn't be possible to do something like make the preferences unusable.

It's not an issue of feasibility/difficulty so much as a "this probably violates Mozilla's add-on policies and/or opens a security hole" issue.

I don't see how it could violate Mozilla's extension policy since addons like Stylus (inject CSS into any arbitrary webpage) and TamperMonkey (inject JavaScript into any arbitrary webpage) exist. Also, since they are allowing userChrome.css and userContent.css which can inject CSS not just into webpages, or extension pages but the core of the browser itself.

In fact, here's an example where it DID open a security hole in Slack

That's interesting but I think it would be pretty difficult to exploit in practice. Also, I don't think there's anything comparable in the Tab Stash interface to "keylog". I mean, I guess the search box and it might be possible to detect if there was a tab with a specific title or URL prefix but people would have to be developing Tab Stash-targeted exploits, knowing if a person just had http://www.blah.com/ in their list would have to be of value and they'd have to get the person to paste in the CSS. It just seems like such an incredibly remote scenario.

Even apart from potential security issues, applying arbitrary CSS opens all kinds of avenues for breaking things in subtle ways, and there is no guarantee that a mod will work from version to version.

Yes, that's fair enough and I don't think anyone using CSS overrides (whether Tab Stash eventually offers one or with Firefox's built in stuff) should have an expectation that their stuff will continue working. It's a thing power users can take advantage of.

I think tweaking the current "Compact" style in small ways probably won't cause any consternation. Current users might even appreciate some of these tweaks.

Alright, in that context do you have an opinion on the screenshot I posted?

Mikhail22 commented 2 years ago

Regarding the collapse/expand vs. rename behavior, and in general, it's more important to me to avoid user confusion (and keep things intuitive even for non-power users)

Maybe it's just me but I'm experiencing the opposite, got used to click-to-open/expand in many different software for ages, like Windows explorer or VS Code tree view, etc, so it became natural. Also Note that Firefox uses same principle in e.g. in the Bookmarks tree. So I don't think it relates to being a power-user in this case. But yes, I DO undertand that renaming should be also available at hand. Press and hold for 2 seconds to rename is also a good alternative and is used in some software quite often.

  • I'd avoid the bold font for the active tab, instead use cell bg highlighting

This unfortunately won't work because it conflicts with selection and hover effects, and I also think the bold font helps to make it easier to find the active tab in the list.

I didn't know I can select items, I've tried to find out but could not. Did you really mean it?

josh-berry commented 2 years ago

I didn't know I can select items, I've tried to find out but could not. Did you really mean it?

Click the item's icon. You can even use Shift+Click to select a range of items (for now within a single group, looking into how to do it across groups soon). :)

josh-berry commented 2 years ago

I don't see how it could violate Mozilla's extension policy since addons like Stylus (inject CSS into any arbitrary webpage) and TamperMonkey (inject JavaScript into any arbitrary webpage) exist. Also, since they are allowing userChrome.css and userContent.css which can inject CSS not just into webpages, or extension pages but the core of the browser itself.

Those are non-privileged pages. Injecting code into privileged extension pages themselves is a whole different ballgame in terms of the amount of data that can be captured (especially considering Tab Stash's rather extensive list of permissions).

In fact, here's an example where it DID open a security hole in Slack

That's interesting but I think it would be pretty difficult to exploit in practice. Also, I don't think there's anything comparable in the Tab Stash interface to "keylog". I mean, I guess the search box and it might be possible to detect if there was a tab with a specific title or URL prefix but people would have to be developing Tab Stash-targeted exploits, knowing if a person just had http://www.blah.com/ in their list would have to be of value and they'd have to get the person to paste in the CSS. It just seems like such an incredibly remote scenario.

The specific example isn't the point; the point is that CSS is an extremely complex standard that generally assumes whatever is written is trustworthy, and if that assumption is violated, there could be unknown/presently-unknowable consequences. The example was just to show that security issues actually have arisen from allowing arbitrary CSS; the concern isn't purely theoretical.

To be clear, it's not that I'm against modding or giving people customization ability (or even that I'm worried about people breaking their setup), it's that I want to avoid unpleasant surprises for users and stay within the bounds of what Mozilla will allow. So at minimum, I want to think very very carefully before opening that can of worms. If there's a way to do it that's reasonably safe (or at least has appropriate warnings/guardrails so people really understand what they're doing) and doesn't break the rules, I'm open to it.

I think tweaking the current "Compact" style in small ways probably won't cause any consternation. Current users might even appreciate some of these tweaks.

Alright, in that context do you have an opinion on the screenshot I posted?

It generally looks OK. Personally I would want a little more margin on either side I think (but maybe not as much as the Compact style currently has), but what you have looks fine. (The folder title and collapse buttons look a little misaligned though.) The search bar also looks really really squished (though again, it could probably be tightened up a little compared to what I have now)—I'm guessing you cut the borders down to zero, and I think I'd leave maybe 1-2px?

Did I miss any other changes? Do you want to just share your mods here? Would probably be easier to tell what's changed.

Also, Hyperbole and a Half is a great webcomic and book, and I wish I had more opportunities in my life to write Rust. 😄

KerfuffleV2 commented 2 years ago

@josh-berry

Injecting code into privileged extension pages themselves is a whole different ballgame in terms of the amount of data that can be captured

That's true. I think the danger of stuff like access remote resources can be removed by locking down the CSP some more.

I messed with this a bit and came up with a proof of concept: #239 — the changes required were surprisingly small. Needs quite a bit more testing/verification/polish before it could be ready to merge (even if you were open to that) but I will say I'm going to be using it myself!

The example was just to show that security issues actually have arisen from allowing arbitrary CSS; the concern isn't purely theoretical.

Noted. I'm not sure there's really a way to make it completely safe for people to just paste in CSS they don't understand from sources they don't trust, because like you said future exploits aren't really knowable at this point. We can reduce the risk (possibly eliminate it in some cases) for known potential issues though and also make sure people are informed about the risk they are exposing themselves to.

So at minimum, I want to think very very carefully before opening that can of worms. If there's a way to do it that's reasonably safe (or at least has appropriate warnings/guardrails so people really understand what they're doing) and doesn't break the rules, I'm open to it.

That's fair, and I appreciate that you're giving it serious consideration. I'm a charge ahead and deploy the fun new feature kind of person, so it's good there's someone more level headed to keep me from doing anything too crazy. :)

I'm also completely fine with any amount of warning disclaimers/confirmation dialogs to enable it (for the first time, at least.)

I don't think it will break any rules but I will see what I can find out to make sure that's 100% the case.

Perhaps we should move further discussion about that to the PR I linked.

Do you want to just share your mods here? Would probably be easier to tell what's changed.

Sure. Just to be clear, I was mostly asking if that was in the scope of changes you'd be looking at for changing the existing compact theme. There are fairly large differences. The actual CSS changes right now are quite haphazard and I wouldn't even submit them as a style people could use with something like the custom style function let alone the official theme.

This is what I'm using right now:

@-moz-document url-prefix("moz-extension://b7b97e77-b199-40c2-a4e2-0fd07f35b385/") {
  .action-container.tab a.text {
    text-overflow: clip !important;
    margin-left: 4px !important;
  }
  .action-container.tab img.action {
    margin: 0px !important;
    padding: 0px !important;
    scale: 0.7 !important;
  }
  .action-container.tab span.action {
    margin: 0px !important;
    padding: 0px !important;
    scale: 0.7 !important;
  }

  :not(.notification) .contents { background: #202020; }
  aside.notification, aside.notification .contents { background: #707080 !important; }
  section.folder { background: #202020; }

  .action-container {
    margin: 0px !important;
    padding: 0 4px !important;
    height: unset !important;
  }
  .folder-item:not(.disabled):hover, .folder-item:not(.disabled):focus-within {
    background-color: #353535 !important;
  }
  .folder:not(.collapsed) span.folder-name {
    color: #b0b0b0;
  }
  .folder.collapsed span.folder-name {
    color: #808080;
  }
  footer { display: none !important; }
}

Also, Hyperbole and a Half is a great webcomic and book,

I saw it a lonnnng time ago and randomly thought of it the other day. Still as good as I remembered! Sadly, I don't have the book (yet.)

and I wish I had more opportunities in my life to write Rust. :smile:

Me too! Definitely my favorite language right now. I've been getting into metaprogramming with proc macros, it's really interesting how much you can do.

KerfuffleV2 commented 2 years ago

Here's probably a better example. Attempt at Solarized Dark:

image

edit: Also now a slight fade effect on the tab titles instead of just hard clipping:

image

gist since the CSS is pretty long: https://gist.github.com/KerfuffleV2/cdca4f3a89e35c0e0216d1f37a9ce259

jstetten commented 2 years ago

Love the add-on, and the discussions here, guys.

Lemme know if you'd like a separate issue, but could I make a small suggestion to just incorporate @KerfuffleV2 's fade out effect (instead of ellipses) throughout in the next release?

  1. It allows displaying of more of the tab title, conveying that much more important information in tight constraints.
  2. Gradients are natural and easy on the eyes; but here they additionally eliminate one more element of unneeded visual clutter, letting the user focus on the task at hand quicker.
  3. It adds a layer of polish to an already nice-looking and time-saving add-on.
tomasklaen commented 1 year ago

I'd like to ask about the sate of this feature request:

You can't drag tabs/bookmarks into collapsed folders.

As since it was posted there was no reaction to it. Is this something planned or maybe a wontfix?

After converting my hundreds of tabs into tab stash, I wanted to categorize them into groups, but without the ability to actually drag into a collapsed group this is incredibly painful.

josh-berry commented 1 year ago

Hi @tomasklaen, you actually don't even need to use drag-and-drop for this. Once you've selected the tabs you want to move, you have two options:

  1. Scroll to the group you want and use one of the blue buttons that appear when you hover over the group title to move the selected tabs into that group
  2. Use the selection menu (dark blue, appears next to the search bar when items are selected) to move the selected tabs directly to the target group.

Hope this helps!