Sync maximized tag with application model also in case of a maximized shell

[vogella]

Before this change the WBWRenderer did not update the size of the application in the application model if the shell was maximized, only if the shell got re-sized.

This lead to outdated information in the application model and forces clients which are interested in the actual size of the Window to check in the Shell instead in the model.

This was introduced by
https://bugs.eclipse.org/bugs/attachment.cgi?id=183776&action=diff. The actual logic of restoring of size is done via a tag, so putting the size in the application should not affect the application behavior.

Fixes #3054

[laeubi]

Will this not result in once maximized the maximized size will be persisted so one cant restore it anymore?
That's also what the comments seem to indicate... Also a maximized size might change when the shell is later moved to a larger (or smaller) monitor...

[vogella]

Will this not result in once maximized the maximized size will be persisted so one cant restore it anymore? That's also what the comments seem to indicate... Also a maximized size might change when the shell is later moved to a larger (or smaller) monitor...

The actual logic of restoring of size is done via a tag, so putting the size in the application does not affect the application behavior.

[laeubi]

The actual logic of restoring of size is done via a tag

How could a tag know the size BEFORE I maximized the shell? This seems impossible to me... can you maybe give some code hints?

Just assume the following case:

1. I have an Application window with 400x400 size
2. Now I maximize the shell (tag gets added)
3. I restart and restore it (tag removed) and end up with 400x400 size again

With this change the size will be set to something different and stored at step (2) then step (3) will not restore it to the previous size...

[vogella]

The actual logic of restoring of size is done via a tag

How could a tag know the size BEFORE I maximized the shell? This seems impossible to me... can you maybe give some code hints?

Just assume the following case:

1. I have an Application window with 400x400 size
2. Now I maximize the shell (tag gets added)
3. I restart and restore it (tag removed) and end up with 400x400 size again

With this change the size will be set to something different and stored at step (2) then step (3) will not restore it to the previous size...

Did you look at https://bugs.eclipse.org/bugs/attachment.cgi?id=183776&action=diff? If the tag is present, shell.setMaximized() is called.

[vogella]

The actual logic of restoring of size is done via a tag

How could a tag know the size BEFORE I maximized the shell? This seems impossible to me... can you maybe give some code hints?
Just assume the following case:

1. I have an Application window with 400x400 size
2. Now I maximize the shell (tag gets added)
3. I restart and restore it (tag removed) and end up with 400x400 size again

With this change the size will be set to something different and stored at step (2) then step (3) will not restore it to the previous size...

Did you look at https://bugs.eclipse.org/bugs/attachment.cgi?id=183776&action=diff? If the tag is present, shell.setMaximized() is called.

The maximize / restore is not handed by the Shell, no application model logic is used here. Maybe that was the missing part? The tag is not involved in this handling.

[laeubi]

If the tag is present, shell.setMaximized()

I understand that but as said this only work in the current session to restore the size of the shell. It does not work after a restart because then the Shell has no way to know a size.

So how it works is:

1. Shell gets "model size"
2. The we set maximized=true
3. Shell is larger now but still has "model size" so it restores to it if we set maximized=false

[laeubi]

Looking at the model code I think the issue is more to be solved either by:

1. getWidth/Heightmust behave like Shell, that is ask for the actual size instead of return what was set via setWidth/Height (was has no effect on maximized windows)
2. One would need a new method getSize() that behaves different from getWidth/Height as described in (1)

So effectively reverting the previous added code seems not appropriate also given that there where no complains about the current behavior for a long time we should assume it was done for a purpose.

[vogella]

If the tag is present, shell.setMaximized()

I understand that but as said this only work in the current session to restore the size of the shell. It does not work after a restart because then the Shell has no way to know a size.

So how it works is:

1. Shell gets "model size"
2. The we set maximized=true
3. Shell is larger now but still has "model size" so it restores to it if we set maximized=false

Thanks, that is a valid scenario which I did not see. Handling the maximized tag would allow the user to access this information, I updated the PR.

[vogella]

Looking at the model code I think the issue is more to be solved either by:

1. getWidth/Heightmust behave like Shell, that is ask for the actual size instead of return what was set via setWidth/Height (was has no effect on maximized windows)
2. One would need a new method getSize() that behaves different from getWidth/Height as described in (1)

So effectively reverting the previous added code seems not appropriate also given that there where no complains about the current behavior for a long time we should assume it was done for a purpose.

I took a third approach, updating the maximized tag during runtime, please have a look if you see issue with that approach.

[laeubi]

Sounds reasonable. It would still be impossible to know the "real" or "native" size of the window so a shell is still needed.

[vogella]

Sounds reasonable. It would still be impossible to know the "real" or "native" size of the window so a shell is still needed.

True, but at least the model data would indicate the maximized state of the Shell and we would have a trigger point to access it (or read the size of the screen. Also, AFAICS the WBWRenderer does currently not react to tag changes so setting the tag during runtime would not re-size the Shell again.

[github-actions]

Test Results

 2 778 files  ±0   2 778 suites  ±0   1h 39m 8s ⏱️ +9s
 7 928 tests ±0   7 697 ✅  - 3  228 💤 ±0  3 ❌ +3 
23 337 runs  ±0  22 588 ✅  - 3  746 💤 ±0  3 ❌ +3 

For more details on these failures, see this check.

Results for commit 607d41e. ± Comparison against base commit ca63f41.