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

docs.google.com - Norwegian characters do not trigger "edit mode" for spreadsheet cells #1879

Closed
hallvors opened this issue Nov 4, 2015 · 8 comments
Milestone

Comments

@hallvors
Copy link

hallvors commented Nov 4, 2015

URL: https://docs.google.com
Browser / Version: Firefox 41.0
Operating System: Windows
Problem type: Something else - I'll add details below

Steps to Reproduce

  1. Navigate to: https://docs.google.com
  2. Open spreadsheet
  3. Focus an empty cell but do not enter "editing mode". (I.e. single-click, not double-click)
  4. Type a Norwegian character like æ, ø or å

Expected Behavior: Spreadsheets should switch to editing mode for the cell automatically and insert the character

Actual Behavior: Nothing happens.

Works in Chrome and IE.

@hallvors hallvors changed the title docs.google.com - see bug description docs.google.com - Norwegian characters do not trigger "edit mode" for spreadsheet cells Nov 4, 2015
@hallvors
Copy link
Author

hallvors commented Nov 4, 2015

The same issue seems to affect the ? key stroke - on Norwegian keyboards this is [Shift] - [+] - BTW the + key is just to the right of the number keys.

@hallvors
Copy link
Author

Re-tested, still happens.

@denschub
Copy link
Member

Alright, sweet. We've got two issues for the price of one.

The initial key code check and assignment to some variables is done in 2473645297-ritz_waffle_i18n_core.js. The code below transforms the event object into some objects that gets used in various places:

function bca(a, b) {
  if (a.Pu) return !0;
  if (!Gba) {
    var c = b || ma("window.event");
    b = new Zc(c, this);
    var d = !0;
    if (!(0 > c.keyCode || void 0 != c.returnValue)) {
      a: {
        var e = !1;
        if (0 == c.keyCode) try {
          c.keyCode = -1;
          break a;
        } catch (h) {
          e = !0;
        }
        if (e || void 0 == c.returnValue) c.returnValue = !0;
      }
      c = [];
      for (e = b.currentTarget; e; e = e.parentNode) c.push(e);
      a = a.type;
      for (e = c.length - 1; !b.O && 0 <= e; e--) {
        b.currentTarget = c[e];
        var g = eca(c[e], a, !0, b), d = d && g;
      }
      for (e = 0; !b.O && e < c.length; e++) b.currentTarget = c[e], g = eca(c[e], a, !1, b), 
      d = d && g;
    }
    return d;
  }
  return fca(a, new Zc(b, this));
}

So, the first issue here is a Gecko issue. For some non-American keys, including ø and ä, we seem to be unable to generate key codes and KeyboardEvent.keyCode/KeyboardEvent.which is 0. I was not the first one to discover this, and bug 1036008 was already created. I added some information and poked a bit, I hope we can sort this out.

The second issue, however, is the use of KeyboardEvent.keyCode. As far as I can tell, KeyboardEvent.keyCode is deprecated and everyone should use KeyboardEvent.key now.

Sadly, that's not a drop-in replacement. Maybe we could suggest generating .keyCode by using KeyboardEvent.key.charCodeAt(0) and some special exceptions for special keys like control/shift/... but that also would be a lot of work for them.

I guess it's best to wait for bug 1036008 and see what we end up with?

@denschub
Copy link
Member

Okay, judging by the reaction in that bug, I don't think we will get a Gecko fix.

@karlcow @miketaylr I am moving this to needscontact although I am not able to make up a clean or easy solution for this.

Basically, we dropped support for KeyboardEvent.keyCode/KeyboardEvent.which in favor of KeyboardEvent.key. "Dropping support" here means we continue to provide the keyCodes we have been sending before, but it looks like there is no chance of adding new ones or fixing issues. The spec actually was a bit confusing since it is explicitly talking about ASCII characters at some point, so I am not even sure what the expected behavior for non-ASCII keys was.

The new way is to use KeyboardEvent.key or .code. However, these are no drop-in replacements since these methods return a string, not the key ID. So basically, there are two options: rewrite all the places where a keyCode is used or write a complex mapper that takes care of converting the key back into the numeric ID.

Googles code is heavily optimized and I am not sure what would be the best solution, so it's probably best to get in touch. Also, I see no chance of getting the behavior adjusted in Firefox.

@karlcow
Copy link
Member

karlcow commented Aug 31, 2016

Google has been contacted through the usual mailing list.

@miketaylr
Copy link
Member

Thanks everyone!

@karlcow karlcow added this to the sitewait milestone Oct 30, 2017
@reinhart1010
Copy link

reinhart1010 commented Dec 12, 2017

I think this issue is fixed.
Issue fixed
Edit: This was done by pasting the characters instead of typing it directly. Re-tested on on-screen keyboard and the bug still occurs.

@webcompat-bot webcompat-bot modified the milestones: sitewait, fixed Dec 12, 2017
@webcompat-bot webcompat-bot modified the milestones: fixed, sitewait Dec 12, 2017
@webcompat-bot webcompat-bot reopened this Dec 12, 2017
@karlcow karlcow modified the milestones: sitewait, fixed Dec 12, 2017
@webcompat-bot webcompat-bot modified the milestones: fixed, sitewait Dec 12, 2017
@denschub
Copy link
Member

Since pathces in bug 1036008 landed yesterday, I am happy to confirm I have tested this in todays nightly and... it's fixed. MDN has some details about the changed behavior, if someone is interested: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode#keyCode_of_punctuation_keys_on_some_keyboard_layout

Closing this as fixed! 🎉 🔥

@denschub denschub removed this from the sitewait milestone Feb 23, 2018
@denschub denschub added this to the fixed milestone Feb 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants