614944 - Use the value instead of label for controller.select() in the DOMWalker

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Adrian Kalla [:adriank]]

By using node.value instead of node.label with the controller.select(), we can get rid of having to read the labels by utils.getEntity().

The tests will have to be updated accordingly.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

That would be a big win!

Comment 2

[Adrian Kalla [:adriank]]

Attachment: patch v1

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 500689 [details] [diff] [review]
patch v1

>diff --git a/firefox/l10nTests/testAccessKeys/test1.js b/firefox/l10nTests/testAccessKeys/test1.js
>-          title : utils.getEntity(dtds, "historyHeader.remember.label")},
>+          title : "remember"},

Beside those changes, can we also remove some of the dtd files from the dtd array?

>     if (activeNode.localName == "menulist") {
>-      if (nodeToProcess.label != idSet.title) {
>+      if (nodeToProcess.value != idSet.title) {

The naming of the property is kinda misleading. Can we change 'title' to 'value'? How would that affect other elements? Can we limit that change for select elements only?

Otherwise I'm still really up for that change.

Comment 4

[Adrian Kalla [:adriank]]

Attachment: patch v2

(In reply to comment #3)
> Comment on attachment 500689 [details] [diff] [review]
> patch v1
> 
> >diff --git a/firefox/l10nTests/testAccessKeys/test1.js b/firefox/l10nTests/testAccessKeys/test1.js
> >-          title : utils.getEntity(dtds, "historyHeader.remember.label")},
> >+          title : "remember"},
> 
> Beside those changes, can we also remove some of the dtd files from the dtd
> array?

right, we can remove 1 dtd

>     if (activeNode.localName == "menulist") {
> >-      if (nodeToProcess.label != idSet.title) {
> >+      if (nodeToProcess.value != idSet.title) {
> 
> The naming of the property is kinda misleading. Can we change 'title' to
> 'value'? How would that affect other elements? Can we limit that change for
> select elements only?

such a change is possible and shouldn't have any side-effects


I have made those two changes in the v2 patch.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/7f1d9558f493 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/83cb4bb4d51e (1.9.2)