Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong UI calculation for horizontal scrollbars since Chrome 105 #53

Closed
jfuerter opened this issue Sep 7, 2022 · 22 comments · Fixed by #56
Closed

Wrong UI calculation for horizontal scrollbars since Chrome 105 #53

jfuerter opened this issue Sep 7, 2022 · 22 comments · Fixed by #56
Labels
bug Something isn't working

Comments

@jfuerter
Copy link

jfuerter commented Sep 7, 2022

Since Chrome 105 (same with Edge 105), as soon as a horizontal scrollbar appears, the UI assigns clicks to the scrollbar not only for the place where the scrollbar actually is present, but also for an area of about the same size below. I've included an example below. We can always reproduce it with the ScrolledForm of a FormPage.

In this screenshot, all the yellow area is actually handling the scrollbar. I only have a small area at the bottom of the pages where I can switch to one of the other pages.
Scrollbar bug

Info:

  • Tested with Eclipse RAP 3.21 (and 3.19)
  • The problem didn't occur before Chrome 105 (we tested it against 104).
  • We use the 'business' theme (maybe the problem doesn't occur with the default theme?)

Here an example how we can reproduce the problem with as little code as possible:

Editor Input

package test;

import org.eclipse.jface.resource.ImageDescriptor;
import org.eclipse.ui.IEditorInput;
import org.eclipse.ui.IPersistableElement;

public class TestEditorInput implements IEditorInput {

    @Override
    public String getName() {
        return "title"; //$NON-NLS-1$
    }

    @Override
    public boolean equals(Object obj) {
        if (obj instanceof TestEditorInput) {
            return true;
        }
        return false;
    }

    @Override
    public <T> T getAdapter(Class<T> adapter) {
        return null;
    }

    @Override
    public boolean exists() {
        return false;
    }

    @Override
    public ImageDescriptor getImageDescriptor() {
        return null;
    }

    @Override
    public IPersistableElement getPersistable() {
        return null;
    }

    @Override
    public String getToolTipText() {
        return getName();
    }
}

Editor

package test;

import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.ui.PartInitException;
import org.eclipse.ui.forms.editor.FormEditor;

public class TestEditor extends FormEditor {

    public static final String ID = TestEditor.class.getName();

    @Override
    protected void addPages() {
        try {
            addPage(new TestPage(this));
            addPage(new TestPage(this));
            addPage(new TestPage(this));
            addPage(new TestPage(this));
            addPage(new TestPage(this));
        } catch (PartInitException e) {
            // ignore for this test
        }
    }

    @Override
    public void doSave(IProgressMonitor monitor) {
        throw new RuntimeException("The method doSave is not yet implemented for class EditorPart"); //$NON-NLS-1$
    }

    @Override
    public void doSaveAs() {
        throw new RuntimeException("The method doSaveAs is not yet implemented for class EditorPart"); //$NON-NLS-1$
    }

    @Override
    public boolean isSaveAsAllowed() {
        return false;
    }
}

Page

package test;

import org.eclipse.swt.SWT;
import org.eclipse.swt.layout.GridData;
import org.eclipse.swt.layout.GridLayout;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Label;
import org.eclipse.ui.forms.IManagedForm;
import org.eclipse.ui.forms.editor.FormEditor;
import org.eclipse.ui.forms.editor.FormPage;
import org.eclipse.ui.forms.widgets.ScrolledForm;

public class TestPage extends FormPage {

    private static final String ID = TestPage.class.getName();

    public TestPage(FormEditor editor) {
        super(editor, ID, "Title"); //$NON-NLS-1$
    }

    @Override
    protected void createFormContent(IManagedForm managedForm) {
        ScrolledForm form = managedForm.getForm();

        Composite body = form.getBody();
        body.setLayout(new GridLayout());

        Label label = new Label(body, SWT.NONE);
        label.setLayoutData(new GridData(6000, -1)); // almost guarantees a horizontal scroll bar
        label.setText("label"); //$NON-NLS-1$
    }
}
@ifurnadjiev
Copy link
Contributor

Just updated to Chrome 105 and I can't reproduce it with our Controls Demo and with Workbench Demo (business theme). I'll give your snippet a try when I have time

@jfuerter
Copy link
Author

jfuerter commented Sep 7, 2022

I'm sorry, I should have mentioned that. I already tried to reproduce it in the Controls and Workbench Demo but couldn't find a similar place. And maybe the problem doesn't affect all ScrolledComposites but only ScrolledForms (as used in my example).

@mknauer mknauer added the bug Something isn't working label Sep 14, 2022
@jfuerter
Copy link
Author

@ifurnadjiev I'm more than willing to debug this problem in order to provide a possible solution for the issue.
To avoid spending hours after hours in the wrong place, could you please give me a hint where to look?

@ifurnadjiev
Copy link
Contributor

Probably you can debug it by inspecting (in Chrome dev tools) first the horizontal scrollbar div height. The client-side handling of the ScrolledComposite is done in Scrollable.js where the two scrollbars are positioned.
Is it possible to create a self-running project to demonstrate the issue?

@jfuerter
Copy link
Author

While debugging in Chrome I thought I'd found the problem. (I had a scroll area with a bottom position of -17 and a height of 708 Pixels, as soon as I changed the bottom position to 0 and the height to 691 Pixels it worked as excepted). But I found out that calculation was the same in Chrome 104 and there it worked quite fine.
Wrong height calculation
Wrong height calculation 2

So I prepared a simple Projekt based on the 'RAP Mail Template' using the classes above. It turned out, the key to the problem is that it only occurs when I'm using the business.css from org.eclipse.rap.design.example_3.22.0.20220617-1351.jar.

Do you want me to upload the sample projekt somewhere or do you prefer to reproduce it with the business.css yourself?

@ifurnadjiev
Copy link
Contributor

Is the .css file the only difference in the mail demo? Probably some CSS definitions in the business.css are not updated accordingly.
Please upload the demo project somewhere in order to get it from there.

@jfuerter
Copy link
Author

jfuerter commented Sep 19, 2022

Here is a link to the demo projekt (only available for 7 days): https://we.tl/t-IbvuSW8pqU
(edit 2022-09-19 @mknauer : attached ZIP archive from the above link as Issue 53.zip)
After starting the webapp you have to klick on the '+' icon to open the editors with the horizontal scrollbars to reproduce the problem:

grafik

Otherwise here the description what I did to the mail template projekt:

The mail template doesn't have the required editor (to reproduce the problem). I've integrated all the classes above in the corresponding packages, extended the plugin.xml by the following:

   <extension
         point="org.eclipse.ui.editors">
		<editor
			class="test.TestEditor"
			default="false"
			id="test.TestEditor"
			name="Test (NON-NLS)">
		</editor>
   </extension>
   <extension
         point="org.eclipse.rap.ui.themes">
      <theme
            file="theme/business.css"
            id="test.theme"
            name="Theme (NON-NLS)">
      </theme>
   </extension>
   <extension
         point="org.eclipse.rap.ui.branding">
      <branding
            id="test.branding"
            themeId="test.theme"
            title="test">
            <additionalHeaders>
				<meta name="robots" content="noindex, nofollow, noarchive, nocache, nosnippet" />
			</additionalHeaders>
      </branding>
   </extension>

Changed the entrypoints definition to use the branding:

   <extension
         id="mailapp.entrypoints"
         point="org.eclipse.rap.ui.entrypoint">
      <entrypoint
            applicationId="Issue_53.mailapp"
            brandingId="test.branding"
            id="mailapp.entrypoint"
            path="/mail">
      </entrypoint>
   </extension>

And you have to add the business.css as well as the required 'img' and 'theme/resources' folders from the mentioned .jar file.

To open the editors I changed the 'OpenViewAction' to open my TestEditor instead of the default one.

@ifurnadjiev
Copy link
Contributor

I can reproduce it with the provided test project. I'll investigate it when the time permits.

@jfuerter
Copy link
Author

Thanks a lot, I'm looking forward to it.

ifurnadjiev added a commit that referenced this issue Sep 21, 2022
For Chrome/WebKit-based browsers use pseudo selector "-webkit-scrollbar"
to specify width/height of the scrollbar.
For Firefox use the new CSS "scrollbar-width" property (defined in the
current CSS draft specification) with value "none".

The above properties do not affect the scroll behaviour, just make
native scrollbars invisible.

Fix #53
@jfuerter
Copy link
Author

Thanks alot for the quick fix! I really appreciate it.

@mknauer mknauer linked a pull request Sep 27, 2022 that will close this issue
@theanuradha
Copy link

@ifurnadjiev how can anyone get it on custom components ? as I have html base custom components and all scrollbars is gone now ? can I use some style to get back default like before inside custom component div ?

@ifurnadjiev
Copy link
Contributor

@theanuradha as the fix for this issue is just pure CSS, you can always change these values to fulfill your needs.

@theanuradha
Copy link

@ifurnadjiev what should I use Im trying but it not get me the native scroll inside div

@theanuradha
Copy link

image

@theanuradha
Copy link

@ifurnadjiev how can I remove this globally or provide custom rwt-index.html

@ifurnadjiev
Copy link
Contributor

@theanuradha
Copy link

@ifurnadjiev When use HEAD_HTML it go before
image

@theanuradha
Copy link

this is what I set to HEAD_HTML

                + "      div::-webkit-scrollbar {\n"
                + "        width: unset;\n"
                + "        height: unset;\n"
                + "      }\n"
                + "      div {\n"
                + "        scrollbar-width: unset;\n"
                + "      }\n"
                + "    </style>"````

@ifurnadjiev
Copy link
Contributor

You can also use org.eclipse.rap.rwt.client.service.ClientFileLoader.requireCss(String) to load CSS at runtime.

@theanuradha
Copy link

@ifurnadjiev ClientFileLoader.requireCss helps with load order but it not override and bring native scroll EG:
image

if I use browser tools to remove original properties then only it show native
image

@ifurnadjiev
Copy link
Contributor

The problem came from the values that you set. You keep width/height to 0.

@theanuradha
Copy link

@ifurnadjiev what I should do to set to native default ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants