Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix a bug and improve flash control #844

Closed
wants to merge 1 commit into from
Closed

fix a bug and improve flash control #844

wants to merge 1 commit into from

Conversation

loyalpartner
Copy link

  1. isEmbed method should great than -1
  2. flash player can blur through document.activeElement.blur()

1. isEmbed method should great than -1
2. flash player can blur through document.activeElement.blur()
@@ -390,6 +390,10 @@ onKeydown = (event) ->
event.srcElement.blur()
exitInsertMode()
DomUtils.suppressEvent(event)
else if isEmbed(event.srcElement) and KeyboardUtils.isEscape(event)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you describe what this does?

@philc
Copy link
Owner

philc commented Apr 23, 2014

Looks promising, and nice bug fix. Can you tell me how this improves the handling of a flash embed by providing an example site and vimium's behavior before & after?

@philc philc added the waiting label Apr 25, 2014
@philc
Copy link
Owner

philc commented Aug 16, 2014

bump

mrmr1993 added a commit to mrmr1993/vimium that referenced this pull request Oct 13, 2014
Conflicts:
	content_scripts/vimium_frontend.coffee
@@ -410,6 +414,7 @@ onKeydown = (event) ->
else if (isShowingHelpDialog && KeyboardUtils.isEscape(event))
hideHelpDialog()


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we lose this extra empty line?

@smblott-github
Copy link
Collaborator

Bump, @loyalpartner. Can you provide an example and explanation, please?

@mrmr1993
Copy link
Contributor

It makes the escape key blur flash objects when one is focused. Without this, the flash object receives the next keystroke, it re-focuses, an re-enables insert mode. Basically, this fixes insert mode for <object>s/<embed>s.

@smblott-github
Copy link
Collaborator

Thanks, @mrmr1993.

Can you give me an example of a site where I can see this effect?

Can you think of any use cases where this is the wrong thing to do? I'm thinking, for example, of that Facebook Messenger case in another issue where Facebook and vimium both want to handle the escape key.

@mrmr1993
Copy link
Contributor

My go-to test was BBC iPlayer, ever since YouTube started supporting HTML5. Any website with embedded flash should work though.

There are probably some examples that would break, but I think the solution I suggested on that issue could work here too.

@smblott-github
Copy link
Collaborator

Fixed isEmbed bug in 1497320.

Regarding ESCAPE to blur embedded objects, there doesn't seem to be a huge demand for this, and it may change the UX in ways we cannot predict. So we'll leave it for now. But thanks, @loyalpartner. Bring it up again if you can make a stronger case and we can get a clearer idea of the risks.

@loyalpartner
Copy link
Author

i'm sorry, i am busy for my work

about this question,i found it works on windows and linux, but doesnt work on mac.

在 2014-10-25 23:25:12,"Stephen Blott" notifications@github.com 写道:

Bump, @loyalpartner. Can you provide an example and explanation, please?


Reply to this email directly or view it on GitHub.

@smblott-github
Copy link
Collaborator

@loyalpartner: The most recent discussion of this is over in #1194. The current status is that it has been withdrawn. See #1194 as to why.

Thanks for the original PR. It would be great if you could contribute to the ongoing discussion of this issue.

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.

4 participants