1081083 - Add getSelectedCell method in widgets.js

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect, P1)

Description

[Daniel Gherasim]

We need for bug 996530 a getSelectedCell() method to get the selected cell from a tree.

Let's add this (across all branches) and we can also include it in the class we'll create in bug 1016246.

Comment 1

[Daniel Gherasim]

Attachment: v1.patch

Here is the method.

Comment 2

[Andrei Eftimie]

Comment on attachment 8503104 [details] [diff] [review]
v1.patch

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

lgtm, with the added item in the jsdoc section, ask a review from Henrik please.

::: lib/ui/widgets.js
@@ +46,5 @@
> +/**
> + * Get the selected Cell
> + *
> + * @param {MozElement} aTree
> + *        Element tree to operate on

Please add a @return statement

Comment 3

[Daniel Gherasim]

Attachment: v1.1.patch

Comment 4

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

Comment on attachment 8504706 [details] [diff] [review]
v1.1.patch

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

::: lib/ui/widgets.js
@@ +59,3 @@
>  // Export of functions
>  exports.clickTreeCell = clickTreeCell;
> +exports.getSelectedCell = getSelectedCell;

We should actually get this all into a Tree class. Please file a mentored bug for that.

Comment 5

[Daniel Gherasim]

(In reply to Henrik Skupin (:whimboo) from comment #4)
> Comment on attachment 8504706 [details] [diff] [review]
> v1.1.patch
> 
> Review of attachment 8504706 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: lib/ui/widgets.js
> @@ +59,3 @@
> >  // Export of functions
> >  exports.clickTreeCell = clickTreeCell;
> > +exports.getSelectedCell = getSelectedCell;
> 
> We should actually get this all into a Tree class. Please file a mentored
> bug for that.

Hey, there is already one filed: bug 1016246.

Comment 6

[Daniel Gherasim]

Comment on attachment 8504706 [details] [diff] [review]
v1.1.patch

Applies cleanly on default, aurora, beta and release.

Comment 7

[Daniel Gherasim]

Attachment: v1.1_esr31.patch

Patch for esr31.

Comment 8

[Daniel Gherasim]

Attachment: v1.2_esr31.patch

Removed extra space.

Comment 9

[Andrei Eftimie]

Comment on attachment 8504706 [details] [diff] [review]
v1.1.patch

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

remote:   https://hg.mozilla.org/qa/mozmill-tests/rev/3c0b8bae9dff (default)
remote:   https://hg.mozilla.org/qa/mozmill-tests/rev/db66d5f1b849 (mozilla-aurora)
remote:   https://hg.mozilla.org/qa/mozmill-tests/rev/6a0c5891f286 (mozilla-beta)
remote:   https://hg.mozilla.org/qa/mozmill-tests/rev/fa55b5797aea (mozilla-release)

Comment 10

[Andrei Eftimie]

Comment on attachment 8505294 [details] [diff] [review]
v1.2_esr31.patch

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

lgtm

Comment 11

[Andrei Eftimie]

Comment on attachment 8505294 [details] [diff] [review]
v1.2_esr31.patch

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

https://hg.mozilla.org/qa/mozmill-tests/rev/36d8bcd21096 (mozilla-esr31)