Skip to content

Added url blacklist to settings panel#27

Closed
ChrisLTD wants to merge 2 commits intotelevator-apps:masterfrom
ChrisLTD:blacklist
Closed

Added url blacklist to settings panel#27
ChrisLTD wants to merge 2 commits intotelevator-apps:masterfrom
ChrisLTD:blacklist

Conversation

@ChrisLTD
Copy link
Copy Markdown
Contributor

@ChrisLTD ChrisLTD commented Nov 7, 2013

Uses modified code from the Vimium plugin. I had to write it using commas as separators since we only get a one line text box for Safari extensions. It does support * as wildcards.

The input box is a bit small on my screen, so I suggest dropping the "(blank for none)" part of the first setting's title:
screen shot 2013-11-07 at 5 25 13 pm

We could include that information in the readme file instead.

@guyht
Copy link
Copy Markdown
Collaborator

guyht commented Nov 9, 2013

One issue I have just realised with this is that it won't play nice with tab back and tab forward as when a tab containing a blacklisted URL comes up, the plugin will be disabled.

I think a lot of people use vimari to cycle through tabs and this would break that functionality.

Going to leave this out of the release for the moment while we think of a solution.

@ChrisLTD
Copy link
Copy Markdown
Contributor Author

ChrisLTD commented Nov 9, 2013

Instead of never letting the keys get bound, we could move the URL check into the keypress event itself?

@ChrisLTD
Copy link
Copy Markdown
Contributor Author

I took a look at how Vimium and VimFX handle blacklists, and it appears they both completely stop reacting to keyboard events on blacklisted sites, even tab switching commands are ignored.

Perhaps we just note that the blacklist will also stop tab switching in the documentation?

@genericsteele
Copy link
Copy Markdown

Coming from Vimium in Chrome, I understand this behavior and know what I'm getting into when I blacklist a site. I think it's a reasonable expectation and trade-off.

@guyht
Copy link
Copy Markdown
Collaborator

guyht commented Nov 13, 2013

Ok. I will merge this over the weekend. If there is a backlash, we can always remove it.

@genericsteele
Copy link
Copy Markdown

That's awesome. Thank you sir. It's definitely an opt-in behavior, so I'd imagine it'll go pretty smoothly.

@ChrisLTD
Copy link
Copy Markdown
Contributor Author

You may want to change the defaults I have in the options. I'll bet a lot of people might confused why their tab switching doesn't work on gmail.

@guyht
Copy link
Copy Markdown
Collaborator

guyht commented Nov 13, 2013

I dont think there should be any 'default' sites. As you say, this can just lead to confusion when it stops working for people on certain pages.

@genericsteele
Copy link
Copy Markdown

Yeah I agree. Default sites could be a bit of a surprise.

@guyht
Copy link
Copy Markdown
Collaborator

guyht commented Nov 24, 2013

Merged

@guyht guyht closed this Nov 24, 2013
@guyht
Copy link
Copy Markdown
Collaborator

guyht commented Nov 24, 2013

Added in v1.10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants