Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Bug 910697 - Shows previous keyboard before displaying correct one #12473

Closed
wants to merge 1 commit into from

Conversation

janjongboom
Copy link
Contributor

Fix for #910697. Basically:

  1. Fix showIME and hideIME as they don't do anything at the moment. Uses visibility rather than display to ensure that we don't need another full reflow of all keys.
  2. Don't call showIME before creating the new keyboard is complete
  3. Don't call hideIME if a focus event comes directly after blur
  4. Test cases around all this

delete this.ime.dataset.hidden;
this.ime.classList.remove('hide');

Choose a reason for hiding this comment

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

It looks like the CSS file does not define a 'hide' class, so this line wasn't doing anything at all...

@bmac
Copy link
Contributor

bmac commented Oct 11, 2013

@janjongboom, you mentioned in the bugzilla issue that there is another bug that fixes the the flickering issue. If that bug is #12499 I'm not sure it addresses the flickering in this patch. That bugfix appears to work with the height of the keyboardFrameContainer while the flickering in this bug is caused by setting visibility hidden on ime element.

If that is not the bug you were talking about feel free to ignore this comment.

};

var hideIME = function km_hideIME() {
this.ime.style.visibility = 'hidden';
this.ime.dataset.hidden = 'true';
Copy link
Contributor

Choose a reason for hiding this comment

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

Any chance we could just style [data-hidden='true'] { visibility: hidden; } rather than adding a direct write to the style property?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree, better not to write to the style property.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this.ime.hidden = true would make it display: none. I changed it to using CSS.

@janjongboom janjongboom closed this Jan 9, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants