WebviewPanel.reveal won't reveals to `SIDE_GROUP` if the specified group does not exist

[Vigilans]

• Existing issues searched: true
• Tested under insider edition: true
• Does this issue occur when all extensions are disabled?: true

• VS Code version: Code 1.32.3 (a3db5be, 2019-03-14T23:43:35.476Z)
• OS version: Windows_NT x64 10.0.18362

Bug Report

Steps to reproduce

1. Create a webview panel with viewColumn set to One
2. Call reveal on the panel with viewColumn set to Two (not exists yet)
3. The panel only reveals to Column One.

Workaround to make it right

1. Create a webview panel with viewColumn set to One
2. Execute workbench.action.focusSecondEditorGroup command to ensure Group Two exists
3. Call reveal on the panel with viewColumn set to Two (exists now)
4. The panel reveals to Column Two.

Contrast Example

showTextDocument

Designed behavior

• When clicking Code Now, a doc will be called by showTextDocument in Column 3.

So:

1. If I have 3 Columns, then doc will be open in Column 3.
2. If I have 2 Columns, but Column 2 is empty, the doc will be open in Column 2.
3. If I have 1 non-empty Column, then doc will be open in Column 2.

This behavior is as by design.

reveal

Designed behavior

• When creating the webview, the panel will be called by reveal to Column 3.
• When clicking Code Now, the panel will be called by reveal to Column 2.

↑ Here, if all 3 columns exists, the panel will be created to Column 3 and moved to Column 2, as by design.

↑ Here, what happened?:

1. If I have 2 non-empty Columns, and creating a new webview, the panel get revealed to Column 2, which should have been in Column 3.
2. If I have 2 non-empty Columns, and clicking Code Now, the panel get revealed to Column 2, which is as by design.
3. If I have 1 non-empty Columns, and creating a new webview, the panel get revealed to Column 1, which should have been in Column 2.
4. If I have 1 non-empty Columns, and clicking Code Now, the panel get revealed to Column 1, which should have been in Column 2.

we only allow revealing text documents in the group one greater than the current number of groups.

So, for WebviewPanel.reveal, it's not one greater than, it's equal to, which is not by design.

[mjbvz]

Does this reproduce in the latest VS Code insiders build?

[Vigilans]

Yes, reproduced in this version:

• VS Code version: Code - Insiders 1.33.0-insider (0ac1114, 2019-04-01T15:51:40.913Z)
• OS version: Windows_NT x64 10.0.17763

[Vigilans]

@mjbvz I've built vscode locally, debugged with the first code demo provided, and finally found the cause of such behavior:

1. In the reveal's RPC entry function:

We pass viewColumn: 1 to showOptions.

public $reveal(handle: WebviewPanelHandle, showOptions: WebviewPanelShowOptions): void {
   const webview = this.getWebview(handle);
   if (webview.isDisposed()) {
       return;
   }

   // ↓ This line
   const targetGroup = this._editorGroupService.getGroup(
       viewColumnToEditorGroup(this._editorGroupService, showOptions.viewColumn)
   ) || this._editorGroupService.getGroup(webview.group || 0);
   
   if (targetGroup) {
       this._webviewService.revealWebview(webview, targetGroup, !!showOptions.preserveFocus);
   }
}

The targetGroup will be set to 0, the same as webview.group. However, the desired target group is 1.

2. In viewColumnToEditorGroup:

export function viewColumnToEditorGroup(editorGroupService: IEditorGroupsService, position?: EditorViewColumn): GroupIdentifier {
    if (typeof position !== 'number' || position === ACTIVE_GROUP) {
        return ACTIVE_GROUP; // prefer active group when position is undefined or passed in as such
    }

    const groups = editorGroupService.getGroups(GroupsOrder.GRID_APPEARANCE);

    let candidate = groups[position];
    if (candidate) {
        return candidate.id; // found direct match
    }

    let firstGroup = groups[0];
    if (groups.length === 1 && firstGroup.count === 0) {
        return firstGroup.id; // first editor should always open in first group independent from position provided
    }

    return SIDE_GROUP; // open to the side if group not found or we are instructed to
}

The postion? parameter is passed with 1, and the function will eventually return SIDE_GROUP.

3. In this._editorGroupService.getGroup:

getGroup(identifier: GroupIdentifier): IEditorGroupView | undefined {
    return this.groupViews.get(identifier);
}

The this.groupViews is just a Map containing 1 element, whose key is 0, whereas the passed identifier equals to SIDE_GROUP, whose value is -2.

It will return undefined without any saying, which just means SIDE_GROUP does not come into any effect :(

4. So, the targetGroup will just fall back to the webview panel's groups, or just the group 0, further proving that SIDE_GROUP didn't do anything here.

const targetGroup = this._editorGroupService.getGroup(
    viewColumnToEditorGroup(this._editorGroupService, showOptions.viewColumn)
) || this._editorGroupService.getGroup(webview.group || 0);

5. Therefore, this._webviewService.revealWebview gets called with two same group, which in essence means reveal won't move to the group which does not exist yet.

It is truly not a designed behavior.

[mjbvz]

Thanks for investigating.

This behavior matches how showTextDocument works: we only allow revealing text documents in the group one greater than the current number of groups. So if you have no groups open, showTextDocument with vscode.ViewColumn.Eight always opens the document in group 1. If you have two groups, then showTextDocument with vscode.ViewColumn.Eight would open in group 3.

Closing this issue as by design. Let me know if I have misunderstood the problem

[Vigilans]

@mjbvz Sorry, but your statement makes sure that it is NOT by design.

---

The designed example is moved to the top floor, please refer to it.

Generally speaking,

we only allow revealing text documents in the group one greater than the current number of groups.

For WebviewPanel.reveal, it's not one greater than, it's equal to, which is not by design.

The code analysis above has found all the problems that results in this bug.