1027552 - Align values in BluetoothAttributeEvent.attrs with webidl

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Jocelyn Liu [:jocelyn] [:joliu]]

Align the character cases with webidl enumeration.

Comment 1

[Jocelyn Liu [:jocelyn] [:joliu]]

Attachment: Bug 1027552 - Patch : Align values in BluetoothAttributeEvent.attrs with webidl

This patch convert the value to lowercase to align with webidl enumeration definition.

Comment 2

[Ben Tian (inactive)]

Comment on attachment 8442720 [details] [diff] [review]
Bug 1027552 - Patch : Align values in BluetoothAttributeEvent.attrs with webidl

Review of attachment 8442720 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth2/BluetoothAdapter.cpp
@@ +840,2 @@
>      if (IsAdapterAttributeChanged(type, arr[i].value())) {
>        SetPropertyByValue(arr[i]);

nit: newline here to separate |SetPropertyByValue| and appending lower-cased attribute name.

@@ +840,4 @@
>      if (IsAdapterAttributeChanged(type, arr[i].value())) {
>        SetPropertyByValue(arr[i]);
> +      ToLowerCase(arr[i].name(), name);
> +      types.AppendElement(name);

Only the first character instead of whole string should be lower-cased, even though current attributes are all single word.

Comment 3

[Jocelyn Liu [:jocelyn] [:joliu]]

Attachment: Bug 1027552 - Patch(v2) : Align values in BluetoothAttributeEvent.attrs with webidl

* Define a macro for converting web enum values to strings

Comment 4

[Ben Tian (inactive)]

Comment on attachment 8443223 [details] [diff] [review]
Bug 1027552 - Patch(v2) : Align values in BluetoothAttributeEvent.attrs with webidl

Review of attachment 8443223 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth2/BluetoothCommon.h
@@ +90,5 @@
> +  uint32_t index = uint32_t(_enum);                                  \
> +  _string.AssignASCII(_enumType##Values::strings[index].value,       \
> +                      _enumType##Values::strings[index].length);     \
> +}
> +

Also append to array in this marco since the string is always appended to an array afterwards. And use |do {...} while(0)| for macro of multiple lines.

Comment 5

[Jocelyn Liu [:jocelyn] [:joliu]]

Attachment: Bug 1027552 - Patch(v3) : Align values in BluetoothAttributeEvent.attrs with webidl

Hi Ben,

Thanks for your comment!
I have updated the patch to address them.

Thanks,
Jocelyn

Comment 6

[Ben Tian (inactive)]

Comment on attachment 8443262 [details] [diff] [review]
Bug 1027552 - Patch(v3) : Align values in BluetoothAttributeEvent.attrs with webidl

Review of attachment 8443262 [details] [diff] [review]:
-----------------------------------------------------------------

r=me. Thanks.

Comment 7

[Jocelyn Liu [:jocelyn] [:joliu]]

Attachment: [Final] Bug 1027552 - Patch: Align values in BluetoothAttributeEvent.attrs with webidl, r=btian

Thanks for your time, Ben!

Comment 8

[Ben Tian (inactive)]

https://hg.mozilla.org/integration/b2g-inbound/rev/58d386cbfe55

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/58d386cbfe55