Skip to content
This repository was archived by the owner on Dec 1, 2017. It is now read-only.

Bug 1261488 - ToC & sidebar proto-impl (rebased & unforked)#376

Merged
dmose merged 18 commits intomozilla:akitafrom
dmose:unforked-toc-sidebar-prototype-1261762
Apr 27, 2016
Merged

Bug 1261488 - ToC & sidebar proto-impl (rebased & unforked)#376
dmose merged 18 commits intomozilla:akitafrom
dmose:unforked-toc-sidebar-prototype-1261762

Conversation

@dmose
Copy link
Member

@dmose dmose commented Apr 22, 2016

This is the first part of doing the unforking and untangling that @Standard8 requested in his review of #329.

@mancas mancas changed the title WIP - Bug 1261488 - ToC & sidebar proto-impl (rebased & partly unforked) Bug 1261488 - ToC & sidebar proto-impl (rebased & unforked) Apr 25, 2016
@dmose
Copy link
Member Author

dmose commented Apr 25, 2016

Interestingly, when I run make test locally, I get different unit test failures (in standlone) than the ones in either of the travis builds (in conversation.jsx). Even more interestingly, running the unit tests on 45.0 Mac has them all passing. At least the Travis stuff probably needs to be fixed before landing.

@dmose
Copy link
Member Author

dmose commented Apr 25, 2016

Also, the UI-showcase is in relatively bad shape (a lot of views are broken). I suspect this doesn't want to block landing, but probably does want to be one of the first things we fix post-landing.

// unregister about: handlers
AboutLoop.conversation.unregister(); // eslint-disable-line no-undef
AboutLoop.panel.unregister(); // eslint-disable-line no-undef
AboutLoop.toc.unregister(); // eslint-disable-line no-undef
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I think this was mentioned in Mark's original review comments. Do we still need to go through and address his original comments now that we've unforked?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see the comment you're talking about

#329

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @dmose is thinking about the comments on #360. I'm happy for those comments to be addressed separately there, landed on master, and then de-forked when merging to akita.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I got confused; never mind!

@mancas mancas force-pushed the unforked-toc-sidebar-prototype-1261762 branch from b3b7f82 to 7522eb5 Compare April 26, 2016 11:01
@mancas
Copy link
Contributor

mancas commented Apr 26, 2016

I did squash of the latests commits to get the unit tests stuff all together in one commit

@mancas mancas force-pushed the unforked-toc-sidebar-prototype-1261762 branch from 7522eb5 to 01e405b Compare April 26, 2016 13:39
sidebarBrowser.setAttribute("disable-global-history", "true");

// XXX something like these seem likely to be required once we
// electrolyze this along with a URI_MUST_LOAD_IN_CHILD in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sidebar is already URI_MUST_LOAD_IN_CHILD. I think this probably doesn't apply, however, lets change the XXX to XXX akita and verify it before too much longer.

Actually I think having the must load in child forces it into a remote browser, but specifying the browser element remote by default optimises the process.

So just update the XXX for now please.

@mancas mancas force-pushed the unforked-toc-sidebar-prototype-1261762 branch from 01e405b to 1f2b5f5 Compare April 26, 2016 14:35
@dmose dmose force-pushed the unforked-toc-sidebar-prototype-1261762 branch from 1f2b5f5 to 5dfefab Compare April 27, 2016 03:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants