> On Aug. 29, 2012, 12:21 a.m., Ryan Baxter wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/oauthpopup/container_oauthpopup.js, > > line 86 > > <https://reviews.apache.org/r/6754/diff/3/?file=146637#file146637line86> > > > > Should the parameters passed to this open be the same as the one above? > > If so can we make this code common between the two? > > Dan Dumont wrote: > There's a slight difference. The one in the CommonContainer mixin calls > the CommonContainer mixed in function by name, which could be modified easily > by providing an overriding mixin rather than having to re-register the rpc > endpoints to override the entire feature. I'm just setting it up for now, it > could be broken down to be way more customizable. > > I wanted to make sure that people who didn't use the common container > weren't broken... are there people who don't use the CC? > > Ryan Baxter wrote: > But both open calls go down the same code path by default right? IE they > both end up calling the "private" open function above. If so why are they > passing different parameters.
By default, yes, however the arguments are different. Which means that if someone overrides or augments the mixin for the container we will call that open/error instead of the default open/error. I made some changes to the code to make it much more modular, but the concept of why we aren't calling the same function is the same, it's a matter of scoping. - Dan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6754/#review10831 ----------------------------------------------------------- On Aug. 29, 2012, 7:11 p.m., Dan Dumont wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6754/ > ----------------------------------------------------------- > > (Updated Aug. 29, 2012, 7:11 p.m.) > > > Review request for shindig. > > > Description > ------- > > The container will now handle the launching of oauth requests and fire onopen > and onclose events back into the requesting gadgets. This will allow > container implementations to override the default behavior and possibly > surface them in liteboxes rather than relying on window.open which is often > prevented by popup blockers. > > > This addresses bug SHINDIG-1864. > https://issues.apache.org/jira/browse/SHINDIG-1864 > > > Diffs > ----- > > http://svn.apache.org/repos/asf/shindig/trunk/UPGRADING 1373834 > http://svn.apache.org/repos/asf/shindig/trunk/features/pom.xml 1373834 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js > 1373834 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/feature.xml > 1373834 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.util.base/base.js > 1373834 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/oauthpopup/container_oauthpopup.js > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/oauthpopup/feature.xml > 1373834 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/oauthpopup/oauthpopup.js > 1373834 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/oauthpopup/oauthpopup-test.js > 1373834 > > Diff: https://reviews.apache.org/r/6754/diff/ > > > Testing > ------- > > Fixed up tests for the changes. > > > Thanks, > > Dan Dumont > >
