Closed Bug 882054 Opened 7 years ago Closed 7 years ago

Cleanup widgets inheritance model mechanism a bit

Categories

(DevTools :: General, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 24

People

(Reporter: vporof, Assigned: vporof)

References

Details

Attachments

(1 file)

While writing [0], it became a bit obvious that the wrapping/inheritance model proposed by MenuContainers can be confusing. This bug takes care of:

1. Uses the addon SDK's inheritance methods instead of the current "ViewHelpers.create" (the implementations are largely similar, but it's a good idea to use the existing mechanisms instead of reinventing the wheel).

2. Renames MenuContainer to WidgetMethods and turns it into a standalone object literal. It was dumb having it as a prototype onto a function.

3. Renames MenuItem to Item and doesn't export it anymore, since it was a private module thing.

4. Renames the node setter/getter to "widget", since it more accurately describes what it is.

[0] https://gist.github.com/victorporof/5749386/edit
Depends on: 823577
Attached patch v1Splinter Review
Mechanical change. PLEASE DON'T BE ALARMED BY THE SIZE OF THE PATCH.
Thank you.
Assignee: nobody → vporof
Status: NEW → ASSIGNED
Attachment #761321 - Flags: review?(past)
Usage changes are best depicted by revision 6 at https://gist.github.com/victorporof/5749386/revisions
Blocks: 850145
Comment on attachment 761321 [details] [diff] [review]
v1

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

I am stating for the record that I didn't look at every test file change, but I have faith in you.

::: browser/devtools/shared/widgets/ViewHelpers.jsm
@@ +1236,3 @@
>     */
>    _indexOfElement: function(aElement) {
> +    for (let i = 0; i < this._itemsByElement.size; i++) {

No more caching of the map size, eh?
Attachment #761321 - Flags: review?(past) → review+
(In reply to Panos Astithas [:past] from comment #3)
> No more caching of the map size, eh?

I started having faith in our js engine.
Priority: -- → P3
Rebased and landed: https://hg.mozilla.org/integration/fx-team/rev/9fddc37d18b0
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/9fddc37d18b0
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 24
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.