1003888 - PanelUI.isReady() should be a getter, not a function.

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

PanelUI.isReady() is a function that returns true if the panel has already ensured that it's ready for use.

A getter would probably be more appropriate.

Comment 1

[Jan Svoboda]

Hi. I would like to start working on this bug as a part of my assignment at school. I have already downloaded and built mozilla-central. Can you please give me some more advice for this bug? Like where to find source code related to this problem? :-) Thank you.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

(In reply to Jan Svoboda from comment #1)
> Hi. I would like to start working on this bug as a part of my assignment at
> school. I have already downloaded and built mozilla-central. Can you please
> give me some more advice for this bug? Like where to find source code
> related to this problem? :-) Thank you.

Hey Jan! Sounds good - I'll be your mentor for this bug. Thanks for stepping up to take it!

The source code you'll need to change is in browser/components/customizableui/content/panelUI.js and browser/components/customizableui/src/CustomizeMode.jsm.

Take a look at the patch in bug 994712 - an isReady() function was added to the PanelUI object, but that functions as a getter, and should probably be a getter.

Let me know if you have any questions on how to proceed - the best way is to check "Need more information from", and then put my email address ([e-mail]) in to the text field next to the dropdown, and then Save Changes. That'll put this bug in my queue to answer the question.

Alternatively, e-mail me, or talk to me in #fx-team on irc.mozilla.org - I'm mconley.

Thanks again!

Comment 3

[vikneshwar]

Attachment: getter-for-isReady

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8420257 [details] [diff] [review]
getter-for-isReady

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

Perfect! Thanks! :D

Comment 5

[Jan Svoboda]

I am little bit confused right now. Is this bug already fixed by vikneshwar as I can see in diff?

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Hrm.

Yes, there appears to have been a collision here. It looks like Jan called this bug, but then vikneshwar produced a patch for it. The patch is correct, so I'm not going to r- it or obsolete it just so that Jan can repeat the work.

So I think this is mostly my fault because I didn't immediately assign this bug to Jan. :/

So thanks for the patch vikneshwar, and my sincerest apologies Jan for not immediately assigning it to you.

Jan - would you like to work on bug 1003204 instead?

Comment 7

[Jan Svoboda]

Well, that's ok. No problem. :-) I didn't start working on this yet so I will start working on the bug 1003204. Thank you. :-)

Comment 8

[Ryan VanderMeulen [:RyanVM]]

vikneshwar, thanks for the patch! However, has this been run through the Try server? If it has, please post a link and re-request checkin. Otherwise, please do so :) Mike can assist you if needed.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Try push:

remote:   https://tbpl.mozilla.org/?tree=Try&rev=5bdbe73f5e17

Doing just mochitest-bc, since that's the only test suite that exercises the panel in any capacity.

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Just setting a reminder to check the try results.

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Try run looks good. Let's pull the trigger.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/1e655ab65bc0

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1e655ab65bc0