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

GWT getAbsoluteLeft/Top incorrect in Chrome 61 #9542

Closed
dtapuska opened this issue Aug 11, 2017 · 26 comments

Comments

@dtapuska
Copy link

commented Aug 11, 2017

GWT version: 2.8.0
Browser (with version): Chrome 62.0.3181.0
Operating System: All


Description

GetAbsoluteLeft/GetAbsoluteTop are incorrect.

This is largely hidden in a variety of browsers:

Firefox overrides these with the standard behaviour:

public int getAbsoluteLeft(Element elem) {
return getAbsoluteLeftImpl(elem.getOwnerDocument().getViewportElement(),
elem);
}

Chrome uses:

public int getAbsoluteLeft(Element elem) {
ClientRect rect = getBoundingClientRect(elem);
double left = rect != null ? rect.getSubPixelLeft()
+ elem.getOwnerDocument().getBody().getScrollLeft()
: getAbsoluteLeftUsingOffsets(elem);
return toInt32(left);
}

which looks at the body element which in Chrome 61 is incorrect. It is somewhat correct for < 61.

Steps to reproduce

Just look at the code :-)

Known workarounds

The standard base really should be looking at document.scrollingElement and if it is defined use that as the element to read the scrollLeft/Top from.

Links to further discussions

For googlers: b/63125202

@tbroyer

This comment has been minimized.

Copy link
Member

commented Aug 20, 2017

hubot pushed a commit that referenced this issue Aug 23, 2017

- Correct implementation of document.getScrollTop/left.
For old browsers (<= ie10), we keep the old logic and relies on a
quirk/strict mode check to find the document scroll element.
For other browsers, we check first of document.scrollingElement exists
and use it computing scrollTop/Left. If not we use document.body for
webkit browser or rely on the quirk/strict mode check for other
browsers.
We default to document.documentElement if all the logic above return
null (SVG document)

- Fix getAbsoluteLeft/Top computation for webkit browsers to rely on the
correct implementation of document.getScrollLeft/Top.

Bug: #9542
Bug-Link: #9542
Change-Id: I02fe4de00a3f646687f0550e603fc7bb4aca1b80
@rhanizar

This comment has been minimized.

Copy link

commented Sep 16, 2017

Would you like please to provide a date when you gonna release that fix? Chrome's stable channel was updated to 61.0.3163.91 Thursday, September 14, 2017, which has broken some GWT widgets (PopupPanel, ...) which still rely on the body scrollTop to do some position calculation.

the below menu which is using the PopupPanel widget is now displayed in an incorrect position in chrome 61 that has just been rolled out
image
instead of:
image

Thanks

@gkdn

This comment has been minimized.

Copy link
Contributor

commented Sep 17, 2017

This is already fixed at the head.

@gkdn gkdn closed this Sep 17, 2017

@tbroyer tbroyer added Category-UI and removed ReviewPending labels Sep 17, 2017

@tbroyer tbroyer added this to the 2.8.2 milestone Sep 17, 2017

@rhanizar

This comment has been minimized.

Copy link

commented Sep 17, 2017

thank you guys for fixing that issue, is the head production-ready? would you tell me please when are you going to release the 2.8.2 that does contain that fix.

thank you

@rhanizar

This comment has been minimized.

Copy link

commented Sep 18, 2017

I managed to "cherry-pick" your fix in a big app that uses GWT 2.7.0 using deferred binding, but I'm still looking forward to have it out of the box in the 2.8.2 release.

@vitalygoji

This comment has been minimized.

Copy link

commented Sep 24, 2017

My 2.7 app is broken by this issue. Could you suggest how to fix it? I looked at gwt 2.7 download but it seems that it was last updated in 2014.
Please, help!
Thank you guys and a big respect to you for the work you are doing!

@ahmadseder

This comment has been minimized.

Copy link

commented Sep 26, 2017

If you are using a version lower than 2.8 then

  • Go to https://github.com/gwtproject/gwt
  • Clone the version you need
  • Go to https://github.com/gwtproject/tools and clone it
  • Set the path for the tools in the environment variables ( Windows )
  • Download apache ant - I used 1.7.1 and it worked , make sure to define ant home in the environment variables
  • Update the method you need
  • From CLI navigate to the gwt directory and runant clean dist-dev

Hope this can help

@vitalygoji

This comment has been minimized.

Copy link

commented Sep 28, 2017

Thank you ahmadseder !
I followed your instructions and fixed that bug. I am not sure though how policy of not fixing critical bugs such as this one in older versions contribute to popularity of GWT...

==== EDIT START =====
The GWT steering committee has edited this comment to remove a racist comment that clearly violates GWT's code of conduct. We tried reaching out to Vitaly first to salvage the situation and unfortunately have not heard back from him.

Our guidelines are outlined in [1].

[1] http://www.gwtproject.org/makinggwtbetter.html#befriendly
==== EDIT END ====

@Abhoryo

This comment has been minimized.

Copy link

commented Sep 28, 2017

Hi @rhanizar, which class do you deferred bind ?
I've tried to do it with DOMImpl classes but I can't because of private base class.

@ezlie-nguyen

This comment has been minimized.

Copy link

commented Oct 5, 2017

@rhanizar I am also on 2.7.0 and need this fix as well. Tried deferred binding but I ran into the same problem that @Abhoryo had. Could you give us more details on how you fixed it?

@Abhoryo

This comment has been minimized.

Copy link

commented Oct 6, 2017

I end up building my own GWT like @ahmadseder suggests.
You should release GWT 2.8.2 ASAP.

tomas-muller added a commit to UniTime/unitime that referenced this issue Oct 6, 2017

GWT getAbsoluteLeft/Top incorrect in Chrome 61
- corrected implementation of document.getScrollTop and document.getScrollLeft
- see gwtproject/gwt#9542 for more details
- this commit contains a fresh build of GWT 2.7.0 with the cherry-picked commit gwtproject/gwt@88a028f
@hkdennis2k

This comment has been minimized.

Copy link

commented Oct 10, 2017

Any chance of a official back-port to the 2.7 branch?

Or a suggested hotfix/workaround (e.g. override in classpath)

@tbroyer

This comment has been minimized.

Copy link
Member

commented Oct 10, 2017

Any chance of a official back-port to the 2.7 branch?

No, we only support the latest version.
2.8.2 is coming (soon), and in the meantime you can use the HEAD-SNAPSHOT from https://oss.sonatype.org/content/repositories/google-snapshots/

Or a suggested hotfix/workaround (e.g. override in classpath)

Classpath shadowing is indeed a (fragile) possibility:

  • extract the 6 needed files from gwt-user-2.7.0.jar into your source tree (putting them in the same com.google.gwt.dom.client package)
  • make the same changes as in 88a028f
    With most build tools, your own code will come first in the classpath, so the copied&patched classes will shadow the ones from gwt-user-2.7.0.jar.

I'm afraid we don't have anything better to offer.

@wslade64

This comment has been minimized.

Copy link

commented Oct 14, 2017

@tbroyer I followed your suggestion but I am a bit stuck.

When I define my .gwt.xml file all source is supposed to be a descendent from the directory where this file is located.
Hence I can not include a src/main/java/com/google/gwt/dom/client
I tried placing the com.google.gwt.dom.client package at that point in the .gwt.xml directory and then use however then I fell down a rabbit hole with package name changes.

@tbroyer

This comment has been minimized.

Copy link
Member

commented Oct 14, 2017

@wslade64 You don't need any gwt.xml, or any change to an existing gwt.xml.

Btw, 2.8.2 is currently in tests before release in the coming days.

@hheckner

This comment has been minimized.

Copy link

commented Oct 16, 2017

Hi,
we also experience problems in our application with PopupPanel. Is there any info you can share about the release date of 2.8.2?
We really appreciate your work!

Best regards
Hannes

@tbroyer

This comment has been minimized.

Copy link
Member

commented Oct 16, 2017

We're currently smoke-testing 2.8.2. Unless we find a blocker, it should be released in the coming days.

@guibertjulien

This comment has been minimized.

Copy link

commented Oct 17, 2017

Hello, i have a workaround, i call this after updating popup position :

PopupPanel popup;

popup.center();
fixedPosition(popup.getElement())

public static void fixedPosition(Element element) {
    if (element.getAbsoluteTop() < 0) {
      element.getStyle().setPosition(Position.FIXED);
      element.getStyle().setZIndex(9999);
    }
  }

i use GWT 2.6.1.

@hheckner

This comment has been minimized.

Copy link

commented Oct 18, 2017

That workaround would probably not work for me as we use

progressPopup.showRelativeTo(xxx);

would it?

@guibertjulien

This comment has been minimized.

Copy link

commented Oct 18, 2017

 private void openPopin(final Widget target) {
    popupPanel.setWidget(popupView);
    popupPanel.showRelativeTo(target);
    Scheduler.get().scheduleDeferred(new Scheduler.ScheduledCommand() {
      @Override
      public void execute() {
        Workaround.fixedPosition(popupPanel.getElement());
        popupPanel.setPopupPosition(popupPanel.getPopupLeft() - (popupView.getOffsetWidth() / 2 - target.getOffsetWidth() / 2), popupPanel.getPopupTop() - (popupView.getOffsetHeight() + target.getOffsetHeight()));
      }
    });
  }
public class Workaround {

  private static final String PATTERN_CHROME_VERSION = "Chrome/(\\d+)";

  public static void fixedPosition(Element element) {
    if (Navigator.getUserAgent() != null) {
      final RegExp regExp = RegExp.compile(PATTERN_CHROME_VERSION);
      final MatchResult matchResult = regExp.exec(Navigator.getUserAgent());
      if (matchResult.getGroupCount() == 2) {
        try {
          final int version = Integer.parseInt(matchResult.getGroup(1));
          if (version >= 61) {
            final Style style = element.getStyle();
            style.setPosition(Position.FIXED);
            style.setZIndex(9999);
          }
        } catch (Exception ignored) {
        }
      }
    }
  }
}

OlliTietavainenVaadin added a commit to OlliTietavainenVaadin/gwt that referenced this issue Oct 19, 2017

- Correct implementation of document.getScrollTop/left.
For old browsers (<= ie10), we keep the old logic and relies on a
quirk/strict mode check to find the document scroll element.
For other browsers, we check first of document.scrollingElement exists
and use it computing scrollTop/Left. If not we use document.body for
webkit browser or rely on the quirk/strict mode check for other
browsers.
We default to document.documentElement if all the logic above return
null (SVG document)

- Fix getAbsoluteLeft/Top computation for webkit browsers to rely on the
correct implementation of document.getScrollLeft/Top.

Bug: #9542
Bug-Link: gwtproject/gwt#9542
Change-Id: I02fe4de00a3f646687f0550e603fc7bb4aca1b80
@amihaiemil

This comment has been minimized.

Copy link

commented Oct 23, 2017

@guibertjulien Unfortunately, your workaround seems to work only if the popup widget is visible when the page loads.

For the widgets that are not visible, that you have to scroll down to see, it still won't work :(

EDIT:

Actually the position: FIXED only seems to solve the problem, for the already visible widgets. The popup initially shows in the right place, but then if I scoll the page (with the popup still visible), it will move up and down the page, together with the scrollbar.

openstack-gerrit pushed a commit to openstack-infra/gerrit that referenced this issue Oct 23, 2017

Upgrade GWT to version 2.8.2
This version includes a fix for incorrect getAbsoluteLeft/Top values in
Chrome 61 and later [1], plus various other bugfixes. See the release
notes [2] for full details.

See also the release notes for 2.8.1 [3] which is also included in this
upgrade.

[1] gwtproject/gwt#9542
[2] http://www.gwtproject.org/release-notes.html#Release_Notes_2_8_2
[3] http://www.gwtproject.org/release-notes.html#Release_Notes_2_8_1

Bug: Issue 7519
Change-Id: If6da895e1349336b8853767a537ed4a09c2773f5
@niloc132

This comment has been minimized.

Copy link
Contributor

commented Oct 24, 2017

@amihaiemil (and others), GWT 2.8.2 has been released with the real fix for this, so no workaround should be required any longer.

@amihaiemil

This comment has been minimized.

Copy link

commented Oct 24, 2017

@niloc132

GWT 2.8.2 has been released with the real fix for this, so no workaround should be required any longer.

Yes, but updating the GWT framework is not such an easy task, unfortunately. Everything needs to be re-tested and a lot of bugs could occur (well, it depends on how well the app was written, right?). I wouldn't go through this just for some info tool-tip buttons...

@niloc132

This comment has been minimized.

Copy link
Contributor

commented Oct 24, 2017

That being the case @amihaiemil the simplest (while fragle, so needs to be undone when you do finally update) and most obviously correct fix is from #9542 (comment)

@amihaiemil

This comment has been minimized.

Copy link

commented Oct 24, 2017

@niloc132 Thanks for the alternative. I think we'll update, after all. Tried today and nothing popped, luckily. I was expecting everything to explode, since current version is 2.4.0...

gbugiel pushed a commit to gbugiel/gwt-2.5.1-patches that referenced this issue Oct 24, 2017

- Correct implementation of document.getScrollTop/left.
For old browsers (<= ie10), we keep the old logic and relies on a
quirk/strict mode check to find the document scroll element.
For other browsers, we check first of document.scrollingElement exists
and use it computing scrollTop/Left. If not we use document.body for
webkit browser or rely on the quirk/strict mode check for other
browsers.
We default to document.documentElement if all the logic above return
null (SVG document)

- Fix getAbsoluteLeft/Top computation for webkit browsers to rely on the
correct implementation of document.getScrollLeft/Top.

Bug: #9542
Bug-Link: gwtproject/gwt#9542
Change-Id: I02fe4de00a3f646687f0550e603fc7bb4aca1b80

(cherry picked from commit 88a028f)

wbadam added a commit to vaadin/gwt that referenced this issue Nov 2, 2017

- Correct implementation of document.getScrollTop/left. (#3)
For old browsers (<= ie10), we keep the old logic and relies on a
quirk/strict mode check to find the document scroll element.
For other browsers, we check first of document.scrollingElement exists
and use it computing scrollTop/Left. If not we use document.body for
webkit browser or rely on the quirk/strict mode check for other
browsers.
We default to document.documentElement if all the logic above return
null (SVG document)

- Fix getAbsoluteLeft/Top computation for webkit browsers to rely on the
correct implementation of document.getScrollLeft/Top.

Bug: #9542
Bug-Link: gwtproject/gwt#9542
Change-Id: I02fe4de00a3f646687f0550e603fc7bb4aca1b80
@daocha

This comment has been minimized.

Copy link

commented Jan 26, 2018

I also did the cherry pick for 2.7.0.
I clone 2.7.0 and do the changes according to 88a028f

Then I compile and use in my own project(I've excluded the official 2.7.0 version). I find it still doesn't fix the problem.

When I call label.getAbsoluteTop(),

In my chrome v60, it returns constant value no matter I scroll or not.

In chrome v63, it returns dynamic value.

I'm expecting it returns a constant value (absoluteTop should be the value from document root), am I right?

hjohannsen added a commit to acrolinx/gwt that referenced this issue Jun 22, 2018

- Correct implementation of document.getScrollTop/left.
For old browsers (<= ie10), we keep the old logic and relies on a
quirk/strict mode check to find the document scroll element.
For other browsers, we check first of document.scrollingElement exists
and use it computing scrollTop/Left. If not we use document.body for
webkit browser or rely on the quirk/strict mode check for other
browsers.
We default to document.documentElement if all the logic above return
null (SVG document)

- Fix getAbsoluteLeft/Top computation for webkit browsers to rely on the
correct implementation of document.getScrollLeft/Top.

Bug: gwtproject#9542
Bug-Link: gwtproject#9542
Change-Id: I02fe4de00a3f646687f0550e603fc7bb4aca1b80
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.