Closed Bug 959142 Opened 10 years ago Closed 10 years ago

Jetpack permanent orange on holly because of reliance on CustomizableUI.jsm import

Categories

(Add-on SDK Graveyard :: General, defect, P1)

defect

Tracking

(firefox28 unaffected, firefox29 fixed, firefox30 unaffected, firefox-esr24 unaffected)

RESOLVED FIXED
mozilla29
Tracking Status
firefox28 --- unaffected
firefox29 --- fixed
firefox30 --- unaffected
firefox-esr24 --- unaffected

People

(Reporter: Gijs, Assigned: zer0)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

I'm guessing the Australis UI components need to be disabled in these cases.
So Matteo was unaware of holly - short intro: we keep a backout branch in case we're not comfortable moving Australis to Aurora on the next merge day. It was used for 28; While we would love for Australis to ship on 29, for now we are still maintaining this branch and we should ensure that it works. See:

https://tbpl.mozilla.org/?tree=Holly
Let's have some starring
So ZER0, are you able to take this? How can we de-orange ourselves here?
Flags: needinfo?(dtownsend+bugmail)
Matteo is working on it
Assignee: nobody → zer0
Flags: needinfo?(dtownsend+bugmail)
Attachment #8359424 - Flags: review?(rFobic) → review?(evold)
Attachment #8359424 - Flags: review?(evold) → review+
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/997b767939c702d707742a77bed9fa5e9bc9eec4
Bug 959142 - Jetpack permanent orange on holly because of reliance on CustomizableUI.jsm import

- Added check for `CustomizableUI` in the UI modules

https://github.com/mozilla/addon-sdk/commit/7a187356a1672f7cdfb37d29574f1aefaf57feaa
Merge pull request #1344 from ZER0/holly/959142

Bug 959142 - Jetpack permanent orange on holly because of reliance on CustomizableUI.jsm import r=@erikvold
Hmm I just noticed that we might need a /^Unsupported Application/ check in the test-ui-frame.js 
 https://github.com/mozilla/addon-sdk/blob/master/test/test-ui-frame.js
also, require()s at the top of those two test files need to be refactored a bit to be included in each test function, because with them in the top scope, when they throw (on Holly), the whole test file fails, and the escape code at the bottom doesn't get run.. 

i started work on this yesterday before Matteo took over, so something like this:

https://github.com/zombie/addon-sdk/commit/36c52a87340fc3081f2e19108407700bd8eb8d1c
(In reply to Tomislav Jovanovic [:zombie] from comment #9)
> also, require()s at the top of those two test files need to be refactored a
> bit to be included in each test function, because with them in the top
> scope, when they throw (on Holly), the whole test file fails, and the escape
> code at the bottom doesn't get run.. 
> 
> i started work on this yesterday before Matteo took over, so something like
> this:
> 
> https://github.com/zombie/addon-sdk/commit/
> 36c52a87340fc3081f2e19108407700bd8eb8d1c

Thanks zombie, do you mind updating this and making a pull?
Flags: needinfo?
Erik the check for frame's test and toolbar's test are no needed, because their modules are loaded on top of the tests, not inside the function, so the `Unsupported Application` is throws as expected. You can manually test it.

The unit test system takes care of this exception, zombie:, so it works as expected in Holly too.

In short: I already tested on Holly and unless other bugs there is no need of a new update for that.
Flags: needinfo?
(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #11)
> Erik the check for frame's test and toolbar's test are no needed, because
> their modules are loaded on top of the tests, not inside the function, so
> the `Unsupported Application` is throws as expected. You can manually test
> it.
> 
> The unit test system takes care of this exception, zombie:, so it works as
> expected in Holly too.
> 
> In short: I already tested on Holly and unless other bugs there is no need
> of a new update for that.

Ah ok let's leave this as it is, we should change the other test files at some point, they make the check seem necessary.
Matteo, I thought this was fixed recently? Should we just close this or is there more to do?
Flags: needinfo?(zer0)
Priority: -- → P1
It's fixed on our repo, I was waiting to be sure the change will get in Holly, and the tree will be green again – or, at least, not depends by this bug!

Wes, do you know when Holly will be in sync with our master?
Flags: needinfo?(zer0) → needinfo?(kwierso)
It'll be in Erik's next uplift to m-c, which will then be merged over to Holly at some later point.
Flags: needinfo?(kwierso) → needinfo?(evold)
(In reply to Wes Kocher (:KWierso) from comment #15)
> It'll be in Erik's next uplift to m-c, which will then be merged over to
> Holly at some later point.

right.
Flags: needinfo?(evold)
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: