It used to work but the zoom in animation stops working if you create a new tab in the tabview UI using group + button or double click on group or empty space

This could be related to Bug 633190.

(In reply to comment #1) > This could be related to Bug 633190. I am pretty sure that it's not related to bug 633190 because I am working on a patch for that.

Comment on attachment 513992 [details] [diff] [review] patch v1 Passed try: http://tbpl.mozilla.org/?tree=MozillaTry&pusher=tim.taubert@gmx.de&rev=92a8f2fed542

Comment on attachment 513992 [details] [diff] [review] patch v1 >+ _init: function TabCanvas__init() { >+ var w = this.canvas.width; >+ var h = this.canvas.height; >+ if (!w || !h) >+ return; >+ >+ let ctx = this.canvas.getContext("2d"); >+ this._fillCanvasBackground(ctx, w, h, "#fff"); >+ }, Is the canvas width or height ever non-zero? This is only called from the TabItem constructor, before it's given any geometry, so I would think it should always be 0. In which case, does this patch actually do anything?

(In reply to comment #5) > Is the canvas width or height ever non-zero? This is only called from the > TabItem constructor, before it's given any geometry, so I would think it should > always be 0. In which case, does this patch actually do anything? The <canvas> element has an explicit size of 300x150 (I just checked) so that's why the patch works. Should we give the canvas an explicit size on _init()?

(In reply to comment #6) > (In reply to comment #5) > > Is the canvas width or height ever non-zero? This is only called from the > > TabItem constructor, before it's given any geometry, so I would think it should > > always be 0. In which case, does this patch actually do anything? > > The <canvas> element has an explicit size of 300x150 (I just checked) so that's > why the patch works. Should we give the canvas an explicit size on _init()? Setting a width/height on the canvas early in its init was one of the things I identified as slow in my work for bug 631747... we previously set the width/height on the canvas twice during TabItem creation... once in TabCanvas_init and one via setBounds. That said, I did not test the new tab creation via UI aspects when writing that patch. Bottom line: please don't reintroduce this bit unless completely necessary.

I agree with mitcho; let's not add to the start-up hit. I think what we need for this bug is a force option to override this check in _update: if (!this.isComplete(tab)) { ... that gets used for the _update calls for zoom. This is a good general principle anyway; when we're going to zoom, we should get the latest regardless of whether it's finished, because that's what you'll be looking at when you arrive at the page.

(In reply to comment #8) > I think what we need for this bug is a force option to override this check in > _update: > > if (!this.isComplete(tab)) { > > ... that gets used for the _update calls for zoom. This is a good general > principle anyway; when we're going to zoom, we should get the latest regardless > of whether it's finished, because that's what you'll be looking at when you > arrive at the page. Damn that's exactly how I started to write this patch and this also works, of course. Alright I'm gonna do this :)

Comment on attachment 514061 [details] [diff] [review] patch v2 Looks good!

Comment on attachment 514061 [details] [diff] [review] patch v2 Note to approvers: Very small and low-risk patch that fixes zooming when creating new tabs.

Passed try: http://tbpl.mozilla.org/?tree=MozillaTry&pusher=tim.taubert@gmx.de&rev=b5bd0105b5ff

Comment on attachment 514061 [details] [diff] [review] patch v2 a=beltzner

Mozilla/5.0 (X11; Linux i686; rv:2.0b13pre) Gecko/20110224 Firefox/4.0b13pre Unable to reproduce issue with latest build.