Allow blur of embedded objects (e.g. flash) and add an option to enable/disable it

[mrmr1993]

This is an extension of #1194, which adds the feature but toggles it with an option in the options page, and defaults it to off.

@smblott-github does this deal with your concerns?

[smblott-github]

@smblott-github does this deal with your concerns?

Not really, I'm afraid. Options are bad. We should reserve new options for when we really, really have to have them.

[mrmr1993]

Rebased.

[maximbaz]

An interesting conversation you guys had, let me summarize to see if I missed something.

Both options have a corner case without a workaround:

• DO blur embeds on ESC and prevent it from reacting on ESC, like described in Allow blur of embedded objects (e.g. flash). #1194
• DON'T blur embeds and loose the ability to close tab with x, move out of tab with J and K, like described in vimium not working for pdf files #1243

I think both issues are valid, and we shouldn't ignore them just because we don't have a nice idea on how to fix them.

New options are evil? OK, let's brainstorm another approach! Here you are a couple from the top of my mind:

• Show a checkbox Vimium captures ESC in the context menu only if there is an embedded object on the page
• Create a way to completely disable vimium for the current tab (with the ability to enable it back).

If I had this issue, where would I personally search for such option first of all?

Whatever we chose, it should be intuitive for a person specifically looking for this option, and transparent for everyone else.

Second thing, I believe vimium should blur embeds by default, just because there is no way out once you enter insert mode. While if you are playing flash game and annoyed that vimium captures the ESC keypress, you are able to disable this.
However, such thinking is based on the fact, that we don't know many scenarios of people using embeds. Perhaps we could even make an experiment -- release a version and see how many people would complain 😄

@smblott-github, @mrmr1993 what do you think?

[mrmr1993]

To my mind, the way to do things is:

• merge [Feature request] Ability to import/export settings #1440 (so we can treat esc the same as any other binding).
• add a way to set passkeys for complex keys (eg. <esc>, <c-a>, etc.). I suggest (optionally) using spaces to delimit passkeys , since
  • we currently can't map the spacebar at all, and
  • if we end up allowing it, we'll probably use <space> or similar rather than .
• always handle esc for embeds (although respect passkey rules, as usual).

Then the whole use case is handled by passkeys (ie. just set <esc> as a passkey), and we have no extra UI.

@z0rch I don't think the option has enough utility to warrant having it in the popup. I agree that it should be on by default, but clearly @smblott-github disagrees.

[maximbaz]

Popup is the first place I would intuitively search for such option, that's why I thought about it. Hiding the option unless there is an embed on the page would reduce the noise.

But anyway, it would be definitely the best if I could specify <ESC> in that popup, then extra checkbox is useless.

It is also interesting to think from the complexity perspective, how difficult it is to complete all the steps you described vs move the checkbox from the options into the popup.
If it is time consuming and/or blocked due to other reasons, perhaps it is reasonable to implement a checkbox as the first version, until we get to the more correct implementation.

[maximbaz]

If this pull request is supposed to be merged after #1369, there is no need in having a new option anymore.

[mrmr1993]

Even simpler; if we merge #1369, we could just un-revert the commit from #1194!

[smblott-github]

Your analysis is about right, @z0rch. Blur on Esc is probably the right thing to do for most users (me included), and most use cases. The problem is that the counter example, although probably uncommon, is crippling.

I'd consider reverting a277fa6 in post-1.46 and seeing whether problems arise. That's the simplest "solution".

[smblott-github]

If we do this, @mrmr1993, then I'd favour no option. I'm not sure it passes the new-option threshold.

[maximbaz]

@smblott-github, just for my personal understanding, what is the general purpose of post-1.46 branch? How many people are using it, how long code remains there before being merged into master?

_{The name post-1.46 does sound a bit weird today, since 1.49 is currently released version 😄}

[smblott-github]

what is the general purpose of post-1.46 branch?

At the moment, I'm only putting small changes with high confidence into master. That way, we can quickly do a new release if a problem emerges. I expect post-1.46 to be merged soon.

The name comes because it was started before the 1.46 release, which quickly became 1.49. post-1.46 allows us to keep working forward without messing up master.

[mrmr1993]

The name comes because it was started before the 1.46 release, which quickly became 1.49. post-1.46 allows us to keep working forward without messing up master.

Why don't we just call it dev? Then we can do all our work there, and then pull across the most stable changes into master, even as we move away from the last release.

[mrmr1993]

Your analysis is about right, @z0rch. Blur on Esc is probably the right thing to do for most users (me included), and most use cases. The problem is that the counter-example, although probably uncommon, is crippling.

I've opened #1372 which enables blurring embeds and allows us to work around counter-examples using passkeys. Is that any good @smblott-github?

[smblott-github]

Closing in favour or #1377.