Skip to content

Remove very old FireFox version checks#10308

Open
zbynek wants to merge 9 commits intogwtproject:mainfrom
zbynek:ff-checks
Open

Remove very old FireFox version checks#10308
zbynek wants to merge 9 commits intogwtproject:mainfrom
zbynek:ff-checks

Conversation

@zbynek
Copy link
Copy Markdown
Collaborator

@zbynek zbynek commented Apr 15, 2026

Removes some very old checks, partial fix for #10295

Copy link
Copy Markdown
Member

@niloc132 niloc132 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a local branch that was also attempting to do this, I'll make a PR to your PR and see if we can't get all the fixes in

Comment thread user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java Outdated
Comment thread user/src/com/google/gwt/dom/client/DOMImplMozilla.java Outdated
niloc132 and others added 2 commits April 21, 2026 21:32
Co-authored-by: Colin Alworth <colin@vertispan.com>
@niloc132
Copy link
Copy Markdown
Member

Take a look at zbynek#6 when you get the chance, I think it completes this work.

Additional check removals, merge up to main
// Works around https://bugzilla.mozilla.org/show_bug.cgi?id=548397
return 0;
}
return parseInt(style.marginLeft, 10) + parseInt(style.borderLeftWidth, 10);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that I think this might be wrong in other browsers - the superclass only returns 0, mozilla is the only impl that actually reads values from the dom. Ditto offsettop below. Probably out of scope here, just seeking to remove this extra null check.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FF implementation matches the name, but the WebKit implementation results in more sensible behavior.

  1. go to https://samples.gwtproject.org/samples/Showcase/Showcase.html#!CwBasicPopup
  2. run document.documentElement.style.margin="100px"; in the console
  3. click the image

Expected: popup centered
Actual: popup too far left in FF (but centered in WebKit)

Copy link
Copy Markdown
Member

@niloc132 niloc132 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Self review" of a sort - I can push you a commit if you'd prefer

Comment thread user/test/com/google/gwt/http/server/RequestBuilderTestServlet.java Outdated
Comment on lines -641 to -645
/**
* Implementation of {@link CellTable} used by Firefox.
*/
@SuppressWarnings("unused")
private static class ImplMozilla extends Impl {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking more carefully, maybe we should keep the class with no contents, as the Trident classes, just in case someone has a custom replace-with

zbynek and others added 2 commits April 22, 2026 20:42
Co-authored-by: Colin Alworth <colin@vertispan.com>
@zbynek zbynek added Category-UI ready This PR has been reviewed by a maintainer and is ready for a CI run. labels Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Category-UI ready This PR has been reviewed by a maintainer and is ready for a CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants