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 FF contenteditable backspace bug. #1502

Merged
merged 5 commits into from
Feb 10, 2014
Merged

Conversation

joecomotion
Copy link

@JetFault
Copy link
Contributor

👍 Fixes Issue for me

@ghost ghost assigned ezequiel Jan 8, 2014
d = e.changedNode;
t = inst.config.doc.createTextNode(' ');
d.appendChild(t);
d.removeChild(t);
Copy link
Contributor

Choose a reason for hiding this comment

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

How come this was removed? Are you assuming this problem isn't occurring anymore, or does _fixGeckoOnBackspace inadvertently fix this same problem?

Copy link
Author

Choose a reason for hiding this comment

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

I was unable to repro this ever, so I'm assuming it's not occurring.

Copy link
Contributor

Choose a reason for hiding this comment

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

All right. In that case, let's simply comment it out... just in case.

Copy link
Contributor

Choose a reason for hiding this comment

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

And because I'm paranoid.

@ezequiel
Copy link
Contributor

ezequiel commented Jan 8, 2014

@joecomotion, @JetFault,

Thanks for submitting this fix, and sorry for the insanely late response.

I've gotten this to work and it looks good. 👍

After the few minor picks above are addressed, I will proceed to merge your branch.


//will fail if onEditorReady not called within 3s
if (!editorReady) {
this.wait(3000);
Copy link
Member

Choose a reason for hiding this comment

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

test.wait() works by waiting for a call to test.resume(). So in this case if onEditorReady is called asynchronously your test will always fail. If something may or may not be asynchronous, I suggest you force the test to be asynchronous:

var test = this;

function onEditorReady() { ... }

setTimeout(function () {
  test.makeEditorWithParaPlugin('<p> content<br></p>', function () {
    test.resume(onEditorReady);
  });
}, 0);

// it's best practice not to set a timeout and let the test environment decide
// which timeout to use because in CI some stuff may take longer
test.wait();

@ezequiel ezequiel merged commit 3dfd41f into yui:dev-master Feb 10, 2014
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