Users opening multi-tabs in browsers - Application.getWindow(

Hi guys,

I’m a little bit confused about Application.getWindow and Windows name (for multi-tab browser applications).

The overriding code example given in the Application.getWindow JavaDoc shows:

w.setName(name);

While the Vaadin book, chapter 10 says:

[i]
"The window name is and must be a unique indentifier for each Window object instance. If you use setName() to set the window name explicitly, as we did above, any browser window that has the same URL (within the same browser) would open the same window object. This is dangerous and generally not recommended, because the browser windows would share the same window object. "

[/i]
I think both (book and Javadoc) together are a little bit inconsistent and should be clarified for the newbie readers/learners.

My (probably wrong) understanding is that since a recent version of Vaadin, the name parameter of Application.getWindow(String name) is modified with a postfix random number (as explained in the JavaDoc of Application.getWindow). Then, w = new Window(); w.setName(name); assigns that random number-name to the new Window and produce the same effect as is w.name was not set and left random.

Am I right?

John.

After a debugging session, here is my understanding:

[size=6]
My Experiment

[/size]
This is my very simple implementation of MyApplication.getWindow.

    @Override
    public Window getWindow(String name) {
        // If a dynamically created window is requested, but it does not exist yet, create it.
        Window win =  super.getWindow(name);
        if (win == null) {  // Means that we are in a new browser tab which needs a new MainWindow instance.
        	win = new MainWindow();  // Thiw Window will have a random generated name that will be associated with its corresponding browser tab.
        	
        	// Add it to the application as a regular  application-level window.
            addWindow(win);
        }
        return win;
    }	

My application is structured this way: there is only one window class: MainWindow.
MainWindow contains the visual header/footer.
MainWindow is a FragmentChangedListener and analyses the uri fragments to modify its central content (between the header and footer).

Examples of URL:
http://localhost:8080/JavaBlackBelt/ui/#user/userid=1234
http://localhost:8080/JavaBlackBelt/ui/#course/courseid=5678

JavaBlackBelt is the name of my web application.
web.xml servlet mapping associate ui/* with the Vaadin servlet.

	<servlet-mapping>
		<servlet-name>vaadin</servlet-name>
		<url-pattern>/ui/*</url-pattern>
	</servlet-mapping>

The first URL shows the user screen (MainWindow rearranges its central content to show the user 1234.
The second URL shows the course screen (MainWindow rearranges its content to show the course 5678.

This is probably pretty common for Vaadin developers.

[size=6]
The Results

[/size]
I’ve put a break point in MyApplication.getWindow method to see the possible values of the name parameter.

[b]

  1. I trigger that url in a new fresh Firefox window:

[/b]http://localhost:8080/JavaBlackBelt/ui/#user/userid=10662988

getWindow is called with name = “397880943”
It does not enter the if block because this is probably the main window created in the application.init() method:

@Override
public void init() {
	MainWindow mainWindow = new MainWindow();
	setMainWindow(mainWindow);
}

It enters the method 3 times with that same name (probably ajax requests of the content).

[b]
2. I open a new firefox tab and trigger that URL:

[/b]http://localhost:8080/JavaBlackBelt/ui/#course/courseid=10663073

getWindow is called with name = “397880943_20712765”
It does enter the if block, as expected, to create a new MainWindow instance.

A second request immediately arrives.
getWindow is called with name = “1851108213”
This could seem very strange and unexpected. But it does not enter the if block and fortunately no 3rd instance of MainWindow will be created (we have only 2 browser tabs).
1851108213 is probably the random name of the new MainWindow, automatically assigned because I’ve not called setName on it.

[b]
3. I click a widget (combobox) in the 2nd browser tab.

[/b]
getWindow is called with name = “1851108213”
It does not enter the if block, as expected.

2 requests arrive for that window name (1 click).

[b]
4. I come back the the browser tab 1 and press F5 (browser refresh).

[/b]
getWindow is called (twice) with name = “397880943”. No new MainWindow created.

Note that to do that experiment, you should restart yout application server and use a new Firefox window. If you re-use a firefox window with “old” Vaadin pages that you refresh, these old pages will understand that they die and send (before dying) a request with a window name assigned when these pages were generated before you restared your application server.

John,

this construction - /appname#xyz does not relate to window name - it relates to an URIFragmentUtility. So, all calls - /appname#xyz , /appname#abc, /appname#def and so on will result in calling your main window. But if you’ll add an URIFragmentUtility onto it, you then can catch uri changes events, so you may react and adjust your main window contents accordingly.

In case of window’s name - another scheme should work (at least it was so in ITMill5 and in Vaadin I did not use window naming features yet): /appname/abc , /appname/def

So, having two windows in your application and setting for them setName(“abc”) and setName(“def”), will let toolkit application to show one or another window. In case of unknown name, say /appname/kkk - a main window will be displayed.

But Im wondering - Is Vaading now really return a random Window name instead of what was set via setName() ?

Dmitri

Hi Dmtri,

About the url structure, you are right. I only want one window class. But I want one window instance per open tab, that’s all the point of the thread. In
another thread
, we discussed together the strategy to have a header/footer page structure and decided to have one Window class with changing content, seemed to be the recommended option (in the Sampler application at least).

About your question: “Is Vaading now really return a random Window name instead of what was set via setName() ?”
No. In my experiments, I do not set the name. I don’t call setName. Because the name is null, Vaadin sets the name after getWindow returns.

John.

Hi John,

Your problem with multiple tabs sounds very familiar to me. My application uses the following implementation of multi-windowing, and it seems to work correctly. If I remember correct, some problems occured if the new window wasn’t named (see lines 12 and 45 below)…


import com.itmill.toolkit.Application;
import com.itmill.toolkit.ui.Window;

public class TestApplication extends Application {
    /**
     * First method to be called, initialises the main window.
     */
    @Override
    public void init() {
        TestWindow win = new TestWindow();
        win.setName("TestWindow");
        init(win);
        setMainWindow(win);
    }

    /**
     * Called when the client requests a new window.
     * 
     * @param name name of the new window
     */
    public TestWindow getWindow(String name) {
        // We check application state here because superclass returns null if a)
        // the application is not running anymore b) it doesn't have a window
        // for the given name
        if (!isRunning()) {
            return null;
        }
        Window w = super.getWindow(name);
        if (w == null) {
            w = createNewWindow(name);
        }
        return (TestWindow) w;
    }

    /**
     * Create a new window and add it to the application. Also gives the window
     * a close listener.
     * 
     * @param name
     * @return
     */
    public TestWindow createNewWindow(String name) {
        final TestWindow newWindow = new TestWindow();
        newWindow.setName(name);
        addWindow(newWindow);
        init(newWindow);

        newWindow.addListener(new Window.CloseListener() {
            public void windowClose(Window.CloseEvent e) {
                Window w = e.getWindow();
                if (newWindow.equals(w)) {
                    newWindow.cleanup();
                    removeWindow(w);
                }

            }
        });
        return newWindow;
    }

    /**
     * Init a new window. Called by {@link #createNewWindow(String)}, this
     * method creates all the necessary layout and data to display the new
     * window.
     * 
     * @param win the new, empty window
     */
    public void init(MotonetWindow win) {
        // ...
    }
}

When the user opens the same application in the new browser tab/window, the Vaadin really adds a postfix random number into the window name. The postfix is added because otherwise there would be two windows with the same name. For instance, in my code above the first window is named as “TestWindow” and the name of second (and the following) window would be something like “TestWindow_12345678”.

… continued

I thought my application worked well, but I was wrong.
I was creating too many instances of the main window, because getName is called in other cases I did not know.

  1. Resources

I see that com.vaadin.terminal.gwt.server.AbstractApplicationServlet.getApplicationWindow() calls application.getWindow(“VAADIN”) when request.getPathInfo() equals “/VAADIN/themes/blackbelt/styles.css”
MyApplication.getWindow() method should obviously not create a new window in that case.
MyApplication.getWindow() calls super.getWindow(“VAADIN”) which returns … null, as if I had to create a new main window (but I should simply ignore).

  1. Modal Sub-Windows

Later, I open a modal floating login sub-window. Then I click the login button of MyLoginWindow (which contains a vaadin com.vaadin.ui.LoginForm).
The same com.vaadin.terminal.gwt.server.AbstractApplicationServlet.getApplicationWindow() calls application.getWindo(“loginHandler”) when request.getPathInfo() equals “/loginHandler”
Again, MyApplication.getWindow() method should not create a new window in that case (super.getWindow() returns null also).

My updated, and ugly, implementation of MyApplication.getWindow():

	/** see ancestor useful JavaDoc */
    @Override
    public Window getWindow(String name) {
    	// We check application state here because superclass returns null if
    	// a) the application is not running anymore
    	// b) it doesn't have a window for the given name
    	if (!isRunning()) {
    	    return null;
    	}
    	
        // If a dynamically created window is requested, but it does not exist yet, create it.
        Window win =  super.getWindow(name);
        if (win == null && !"VAADIN".equals(name) && !"loginHandler".equals(name)) {  // Means that we are in a new browser tab which needs a new MainWindow instance.
        	win = new MainWindow();  // This Window will have a random generated name that will be associated with its corresponding browser tab.
        	
        	// Add it to the application as a regular  application-level window.
            addWindow(win);
        }
        return win;
    }

I don’t understand everything. My first conclusion was that the documentation should be improved.

Instead, my current opinion is that the API should be improved to easily and with no risk of programming error, enable multi-tab browsing, which is the only reason why I started overriding Application.getWindow().
I would just call something like this.setMainWindowMultiInstancesPerUser(true) in Application.init().

It’s not urgent but I beleive it’s important. Could the Vaadin dev member concerned by that area of code give his opinion when back from holiday?

Many thanks.
John.

… continued.

I’ve implemented the new API that I suggest.
This code is supposed to go into com.vaadin.Application class.


	private boolean multiTabBrowsing = false;
	
	public boolean isMultiTabBrowsing() {
		return multiTabBrowsing;
	}

	/** When multiTabBrowsing is enabled, the Application.getWindow() method will create a new instance of your main window 
	 * (your current Application.getMainWindow()), for each browser tab/window that the user opens on the client.
	 * If you don't implement multi-tab browsing, your Vaadin application will probably produce "Out of sync" errors for users
	 * who try to open multiple browser tabs/widnows.
	 */
	public void setMultiTabBrowsing(boolean multiTabBrowsing) {
		this.multiTabBrowsing = multiTabBrowsing;
	}



	/** see ancestor useful JavaDoc */
    @SuppressWarnings("unchecked")
	@Override
    public Window getWindow(String name) {
    	// We check application state here because superclass returns null if
    	// a) the application is not running anymore
    	// b) it doesn't have a window for the given name
    	if (!isRunning()) {
    	    return null;
    	}
    	
        // Is that an existing window?
        
        Window existingWindow = (Window) windows.get(name); // Gets the window by name
        if (existingWindow != null) {
        	return existingWindow;

        } else if (! multiTabBrowsing) {  // Legacy case when multiTabBrowsing did not exist. Maybe an override method will create a new window.
        	return null;
        	
        } else {  // New case where we create one instance of the main window per browser window/tab that the user opens in the same HttpSession (= in the same Application instance).
        	///// Is it a request for a new main window (from another browser tab, for the same user/session) ?
        	///// If the name is like MainWindowName_01234567890 (where the number is random), then it is a request for a new instance of main window.
        	
        	Window primaryMainWindow = getMainWindow();
        	if (primaryMainWindow == null) {
        		// We may be in a strange case where Application.getWindow() is called before Application.setMainWindow() has been called. Would it be a bug?
        		return null;
        	}
        	
        	String[] nameParts = name.split("_");
        	if (nameParts.length != 2 || !nameParts[0]
.equals(primaryMainWindow.getName())) {
        		// Name is not like MainWindowName_01234567890
        		return null;
        	}
        	
            /////// We create a new instance of a main window.
        	Class<Window> clazz = (Class<Window>)primaryMainWindow.getClass();
        	Window additionalMainWindow; 
        	try {
        		additionalMainWindow = clazz.newInstance();
        	} catch(Exception e) {
        		throw new RuntimeException("When multiTabBrowsing is enabled, your main window should have a no arg constructor to enable Vaadin to instantiate it.", e);
        	}
        	additionalMainWindow.setName(name);  // we could leave the name null and it would be set to a random number "012345679", but the form "MainWindowName_0123456789" is probably clearer when debugging.
        	addWindow(additionalMainWindow);
        	return additionalMainWindow;
        }
    }	

But, I’ve see something odd (which is not caused by this code).
Open 1 tab: Application.getWindow(“MainWindow”) is called (where “MainWindow” is the name of your main window).
Open a second tab: Application.getWindow(“MainWindow_1111111”) is called as expected (1111111 is a random number). A second instance of MainWindow is created by my code.

In one of the tabs, open a modal window (as my login window), then close it: Application.getWindow(“MainWindow_22222222”) is called where 2222222 is another random number. Isn’t this a bug?. My code thinks it’s a 3rd tab and creates a 3rd instance of MainWindow.

Hi guys,

We slowly reach the end of the summer holidays, and I guess that your team members are coming back.
Who do you think is the most suitable to discuss this matter with me and when do you expect him to be back?

Many thanks.
John.

Hi John. I fully agree that the API is confusing and should be documented better. Unfortunately changing the API and maintaing backwards compatibility is really hard. (Yes, I am the one to blaim for this API - not the original and the “extended” one ;)).

I did not fully go through your proposal, but noticed one major problem with it: it expects that the main window is subclassed and it has a constructor that can create another fully working copy of the main view. Unfortunately this assumption might not be correct.

Some of the usecases we must support:

  • Application maintains only one window (when you open another one, the old one will and should cease working)
  • You can open multiple copies of the “main view” - each has a corresponding window instance in server
  • One can open views to application through links that parametrize the views (for examples ticket numbers in a bug tracking system)

Also if we restrict the programming style for constructing windows somehow - it should be reflected in the API.

Currently my intuition is to keep the current API, but document it better. As I wrote the getWindow() javadoc (
http://vaadin.com/api/com/vaadin/Application.html#getWindow(java.lang.String)
), I am fairly blind for seeing what is confusing and wrong about it. All proposals to make this complex API more comprehensible are welcome.

Thank you for replying Joonas, this is a difficult thread. And I’m afraid it might be an important one needing an extended read.

I think there are two discussions and a half:
1- Should Vaadin support multi tab browsing easily?
2- If yes, how would the API be extended?
3- Isn’t there a bug in how it currently works?

The thrid is the most important to me (because I’ve made the effort of understanding the thing, so the first is more for the newcomers).


1- Should Vaadin support multi tab browsing easily?

I believe it’s more than a documentation problem: as a “framework user” I think that I write too much code that is close to the internals and (that code of mine is) buggy. I admit that in my jounrey with Vaadin, it’s one of the rare case I’ve to go so close from analyzing Vaadin source code. Maybe reading all the posts of this thread will give you an idea of the documentation understanding path for newbies like me, and what’s missing.

Having an web application supporting multi-tab browsers like any other website is, I guess, a very common requirement. I would even bet that it’s the choice of most Vaadin newcomers (who know other technologies for developing web sites).

The good news is that extending the API to make that requirement easily working, is probably an easy/fast development.


2- If yes, how would the API be extended?

For the moment, Vaadin gets the instance of the main window through:

Application.setMainWindow(Window)

Which is limiting to the “Only one browser tab per user” scenario.
I agree with you that the way I get multiple instances of the main window is limiting:

additionalMainWindow = clazz.newInstance();

It should not be hard to provide the user a way to give that instance. It’s probably the original purpose of Application.getWindow(String name).
But now, this Application.getWindow(String name) is too generic, it’s called for any window, new or existing, floating or not. Shouldn’t Vaadin also provide an easier entry point? Most poeple would extend that easier entry point instead of the getWindow(String name).

It could be, for example (and you will probably find a better name):

Window Application.getNewTabMainWindow()

or .getAdditionalMainWindow() or whatever.

As a Vaadin user, I would just have to do:


// In the Application constructor:
this.setEnableMultiTabBrowsing(true);

// In the Application sub-class:
@Override
protected Window getNewTabMainWindow() {
	return new MainWindow();
}

I would not override getWindow anymore and even not read it’s documentation :wink:
Would this harm backward compatibility?


3- Isn’t there a bug in how it currently works?

As described in a previous post of this thread, the combination of multi-tabs and modal windows lead to strange calls that documentation could probably not explain (else I’m eager to have the hint).
That one would prevent my project to go in production.

Do you want a sample code that reproduces the problem or does it work as expected?

Thank you very much for your time, Joonas.

John.

Defitinely yes. I know that this API is problematic and would like to see a solution that would both make the API intuitive as well as maintain backwards compatibility. The best case would be to make it “work out of the box by default”.

I’ll get back to you asap with this.

Here is a code example:


package com.example.wintest;

import com.vaadin.Application;
import com.vaadin.terminal.ExternalResource;
import com.vaadin.ui.Button;
import com.vaadin.ui.Window;
import com.vaadin.ui.Button.ClickEvent;

public class WintestApplication extends Application {

    public void init() {
        setMainWindow(createNewWindow());
    }

    public Window getWindow(String name) {
        Window w = super.getWindow(name);
        if (w == null) {
            w = createNewWindow();
            if (w != null) {
                w.setName(name);
                addWindow(w);
                w.open(new ExternalResource(w.getURL().toString()));
            }
        }
        return w;
    }

    public Window createNewWindow() {
        final Window w = new Window("Browser window "
                + System.currentTimeMillis() % 1000);
        w.addComponent(new Button("Add new subwindow",
                new Button.ClickListener() {

                    public void buttonClick(ClickEvent event) {
                        Window sw = new Window("Subwindow "
                                + System.currentTimeMillis() % 1000);
                        w.addWindow(sw);
                    }
                }));
        return w;
    }
}

In the above example createNewWindow() is just an example on how to create a new window suitable to be shown as main window or in a new tab. Just to demostrate that “subwindows” do not conflict with “browserwindows” i added a button that opens subwindows.

getWindow() method creates new windows when you open new tabs/windows from browser. All the window states should maintained correctly.

Even though the implementation of getWindow() above is short, it is not trivial (or intuitive) at all. This is a problem that could be resolved with better documentation or additional API

API. If we would add createNewWindow() {return null;} to com.vaadin.Application, the getWindow() code could be added to the current implementation. This would not change the default behavior, but supporting simultaneous browser windows / tabs would be as easy as implementing createNewWindow().

Problems with this API:

  • createNewWindow is not a good name. It does not explain fully what happens and when it gets called
  • this does not solve all use-cases. For example that http://my.issue.tracker/issue-32787/

Now i do not follow. Please give a code example for this.

Hi Joonas,

Thank you for your answers.

I’ve tried to reproduce the problem and I could not. Your simple code (from the forum and from the JavaDoc) just work nicely. I see no need to change the API/JavaDoc.

May be except…

While doing the tests, I open a modal sub window containing the com.vaadin.ui.LoginForm.
With this specific form on the window makes Application.getName called with strange values:

  • “VAADIN” when the window opens.
  • “loginHandler” when the form is submitted.

Your code does not handle these cases, you may add to the JavaDoc a warning to the users. That’s why I check the name structure in my code (given in a previous post): “MainWindowName_0123456789” before instantiating a new main window. Or maybe you can avoid that getName is called with these values (nothing is found for them in super.getWindow anyway…).

It brings another question: how to get a reference to the “concerned” main window in the LoginHandler (because geWindow(name) is not called with the main’s window name on sumbit). But I’ll open another form thread for that.

Note for the many readers: I detected a mistake in my code (not shown on previous posts).
To open a new modal sub-window I did:

MyApplication.getCurrent().getMainWindow().addWindow( new MySubWindow() );

(where getCurrent() is a ThreadLocal pattern to get the current Application instance from anywhere),
which is very bad, because it targets always the same instance of MainWindow, instead of the instance corresponding to the browser’s tab of the request.

As a solution, I implemented the threadLocal pattern for the mainWindow as well. I don’t get the value in the TransactionListener (used for the Application threadLocal init) but in … Application.getWindow(name) :wink:
But it’s broken for the LoginForm (because in that very specific case, getWindow(name) is not called with the main window’s name as told to Joonas a few lines above): during the request lifecycle of the submit, the threadLocal value to the mainWindow is not set, and I cannot update the user’s name on the page :-(.
Let’s open another thread for that.

This sounds like a bug in LoginForm. Could you create a ticket with test case for it?