Commit 19b97aeb authored by Chris H-C's avatar Chris H-C
Browse files

Bug 1790637 - Accept camelCase extra keys in JS r=janerik 

Differential Revision: https://phabricator.services.mozilla.com/D179390
parent 14d79275

toolkit/components/glean/bindings/private/Event.cpp

+20 −1
Original line number Diff line number Diff line
@@ -20,6 +20,22 @@ namespace mozilla::glean { 
NS_IMPL_CLASSINFO(GleanEvent, nullptr, 0, {0})

NS_IMPL_ISUPPORTS_CI(GleanEvent, nsIGleanEvent)



// Convert all capital letters to "_x" where "x" is the corresponding lowercase.

nsCString camelToSnake(const nsACString& aCamel) {

  nsCString snake;

  const auto* start = aCamel.BeginReading();

  const auto* end = aCamel.EndReading();

  for (; start != end; ++start) {

    if ('A' <= *start && *start <= 'Z') {

      snake.AppendLiteral("_");

      snake.Append(std::tolower(*start, std::locale()));

    } else {

      snake.Append(*start);

    }

  }

  return snake;

}



NS_IMETHODIMP

GleanEvent::Record(JS::Handle<JS::Value> aExtra, JSContext* aCx) {

  if (aExtra.isNullOrUndefined()) {
@@ -56,6 +72,9 @@ GleanEvent::Record(JS::Handle<JS::Value> aExtra, JSContext* aCx) { 
      return NS_OK;

    }



    // We accept camelCase extra keys, but Glean requires snake_case.

    auto snakeKey = camelToSnake(jsKey);



    JS::Rooted<JS::Value> value(aCx);

    if (!JS_GetPropertyById(aCx, obj, ids[i], &value)) {

      LogToBrowserConsole(
@@ -84,7 +103,7 @@ GleanEvent::Record(JS::Handle<JS::Value> aExtra, JSContext* aCx) { 
      return NS_OK;

    }



    extraKeys.AppendElement(jsKey);

    extraKeys.AppendElement(snakeKey);

    extraValues.AppendElement(jsValue);

    telExtras.EmplaceBack(Telemetry::EventExtraEntry{jsKey, jsValue});

  }

toolkit/components/glean/tests/xpcshell/test_Glean.js

+12 −0
Original line number Diff line number Diff line
@@ -148,6 +148,18 @@ add_task(async function test_fog_event_works() { 
  // Unchanged number of events

  Assert.equal(1, events.length, "Recorded one event too many.");



  // camelCase extras work.

  let extra5 = {

    extra3LongerName: false,

  };

  Glean.testOnlyIpc.eventWithExtra.record(extra5);

  events = Glean.testOnlyIpc.eventWithExtra.testGetValue();

  Assert.equal(2, events.length, "Recorded one event too many.");

  expectedExtra = {

    extra3_longer_name: "false",

  };

  Assert.deepEqual(expectedExtra, events[1].extra);



  // Invalid extra keys don't crash, the event is not recorded,

  // but an error is recorded.

  let extra3 = {