Last Comment Bug 688389 - Multiple tabs in Pop-up windows (created through Panorama)
: Multiple tabs in Pop-up windows (created through Panorama)
Status: VERIFIED FIXED
[fixed-in-fx-team]
: regression
Product: Firefox Graveyard
Classification: Graveyard
Component: Panorama (show other bugs)
: Trunk
: All All
: -- normal
: Firefox 10
Assigned To: Raymond Lee [:raymondlee]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-22 01:45 PDT by AndreiD[QA]
Modified: 2016-04-12 14:00 PDT (History)
8 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
v1 (1.36 KB, patch)
2011-10-03 00:47 PDT, Raymond Lee [:raymondlee]
ttaubert: review+
Details | Diff | Splinter Review
Patch for checkin (1.62 KB, patch)
2011-10-04 10:11 PDT, Raymond Lee [:raymondlee]
no flags Details | Diff | Splinter Review

Description AndreiD[QA] 2011-09-22 01:45:44 PDT
Build ID: 
Win: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110920 Firefox/9.0a1
Linux: Mozilla/5.0 (X11; Linux x86_64; rv:9.0a1) Gecko/20110920 Firefox/9.0a1
Mac: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0a1) Gecko/20110921 Firefox/9.0a1

STR:
1. Launch Firefox on a clean profile and go to http://www.popuptest.com/popuptest1.html - if the case, set permissions to allow pop-ups.
2. Resize one of the open pop-up windows, making it bigger
3. Using the ctrl+shift+e/ cmd + shift +e enter panorama. 
4. While in panorama create another empty group and click over it to create a tab

Actual result:
4. In step 4, a new tab is created; in the newly created tab, some browser functions are invalid (create a new tab, type a weblink, et cetera)

Expected result:
4. In step 4, a new group AND/OR tab should not be created since the window is not a normal browser one (pop-up)


*Notes:
This bug is reproducible on Linux and Mac as well, so setting OS to all.
I tested this on the Firefox channels on Windows and the bug is reproducible also on:
Beta: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0) Gecko/20100101 Firefox/7.0
Aurora: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a2) Gecko/20110918 Firefox/8.0a2
You can find a screen cast video of the issue at the provided link:
https://skydrive.live.com/?cid=bc75b2c6fced798f&id=BC75B2C6FCED798F%21344#cid=BC75B2C6FCED798F&id=BC75B2C6FCED798F%21345
Comment 1 AndreiD[QA] 2011-09-22 07:27:07 PDT
Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=422bbd8245a7&tochange=1a89509e25e4

Last good nightly: 2011-03-31
First bad nightly: 2011-04-01
Comment 2 Raymond Lee [:raymondlee] 2011-09-29 20:21:56 PDT
In popup windows, many things are disabled and hidden e.g. sidebar, menu bar, etc.  IMO, it makes more sense to not allowing user to enter Panorama since there would only be one tab in a popup window.

Thoughts?
Comment 3 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-10-01 01:40:34 PDT
(In reply to Raymond Lee [:raymondlee] from comment #2)
> In popup windows, many things are disabled and hidden e.g. sidebar, menu
> bar, etc.  IMO, it makes more sense to not allowing user to enter Panorama
> since there would only be one tab in a popup window.

Totally agreed, we shouldn't allow Panorama to be loaded in a popup window.
Comment 4 Raymond Lee [:raymondlee] 2011-10-03 00:47:47 PDT
Created attachment 564134 [details] [diff] [review]
v1

Trivial patch
Comment 5 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-10-04 04:50:55 PDT
Comment on attachment 564134 [details] [diff] [review]
v1

Review of attachment 564134 [details] [diff] [review]:
-----------------------------------------------------------------

Great, thanks!

::: browser/base/content/browser-tabview.js
@@ +174,5 @@
>    // If the frame already exists, calls the callback immediately. 
>    _initFrame: function TabView__initFrame(callback) {
>      let hasCallback = typeof callback == "function";
>  
> +    // prevent frame to be initialized for popup window

Nit: "for popup windows".
Comment 6 Raymond Lee [:raymondlee] 2011-10-04 10:11:56 PDT
Created attachment 564598 [details] [diff] [review]
Patch for checkin

(In reply to Tim Taubert [:ttaubert] from comment #5)
> ::: browser/base/content/browser-tabview.js
> @@ +174,5 @@
> >    // If the frame already exists, calls the callback immediately. 
> >    _initFrame: function TabView__initFrame(callback) {
> >      let hasCallback = typeof callback == "function";
> >  
> > +    // prevent frame to be initialized for popup window
> 
> Nit: "for popup windows".

Fixed

Pushed to try and waiting for the results
https://tbpl.mozilla.org/?tree=Try&rev=142447f72b41
Comment 7 Raymond Lee [:raymondlee] 2011-10-04 22:40:17 PDT
Passed Try!
Comment 8 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-10-05 02:11:26 PDT
https://hg.mozilla.org/integration/fx-team/rev/aa487dd537e3
Comment 9 Rob Campbell [:rc] (:robcee) 2011-10-06 04:23:05 PDT
https://hg.mozilla.org/mozilla-central/rev/aa487dd537e3
Comment 10 AndreiD[QA] 2011-10-10 02:02:16 PDT
The issue is not reproducible anymore on the latest Nightly, due to the fix, on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111009 Firefox/10.0a1 (Win)
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0a1) Gecko/20111009 Firefox/10.0a1 (Mac)
Mozilla/5.0 (X11; Linux i686; rv:10.0a1) Gecko/20111009 Firefox/10.0a1 (Linux)

A user cannot enter Panorama using the shortcut keys (ctrl+shift+e -win and linux) and cmd+shift+e on Mac in a pop-up windows (following the steps in the Description)

Note You need to log in before you can comment on or make changes to this bug.