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)
Add-on SDK Graveyard
General
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.
Reporter | ||
Comment 1•10 years ago
|
||
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
Comment 3•10 years ago
|
||
So ZER0, are you able to take this? How can we de-orange ourselves here?
Updated•10 years ago
|
Flags: needinfo?(dtownsend+bugmail)
Comment 4•10 years ago
|
||
Matteo is working on it
Assignee: nobody → zer0
Flags: needinfo?(dtownsend+bugmail)
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8359424 -
Flags: review?(rFobic)
Assignee | ||
Updated•10 years ago
|
Attachment #8359424 -
Flags: review?(rFobic) → review?(evold)
Updated•10 years ago
|
Attachment #8359424 -
Flags: review?(evold) → review+
Comment 6•10 years ago
|
||
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
Comment 7•10 years ago
|
||
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
Comment 8•10 years ago
|
||
Also https://github.com/mozilla/addon-sdk/blob/master/test/test-ui-toolbar.js
Comment 9•10 years ago
|
||
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
Comment 10•10 years ago
|
||
(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?
Assignee | ||
Comment 11•10 years ago
|
||
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?
Comment 12•10 years ago
|
||
(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
Assignee | ||
Comment 14•10 years ago
|
||
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)
Comment 16•10 years ago
|
||
(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)
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
status-firefox28:
--- → unaffected
status-firefox29:
--- → fixed
status-firefox30:
--- → unaffected
status-firefox-esr24:
--- → unaffected
Target Milestone: --- → mozilla29
You need to log in
before you can comment on or make changes to this bug.
Description
•