Bug 78032 – Rename Decimal::ceiling() to Decimal::ceil()

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Eric Seidel (no email)]

Remove wtf_ceil helper

http://trac.webkit.org/changeset/37498

Mac OS X must have been fixed by now?

[Eric Seidel (no email)]

Created attachment 125925 [details]
Patch

[Eric Seidel (no email)]

This came up when reviewing one of Levi's patches where he happened to type the word "ciel" and had to use and #ifdef to make it wtf_ceil on Darwin.  (obviously there were other solutions to that problem, but it seemed we could likely remove this now anyway).

[WebKit Review Bot]

Attachment 125925 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 1 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Seidel (no email)]

Created attachment 125939 [details]
Patch

[Darin Adler]

How can you decide it’s fixed without testing?

[Eric Seidel (no email)]

(In reply to comment #5)
> How can you decide it’s fixed without testing?

Well, the original change added tests (which still seem to pass after this change).  The original change also said:

"The math functions are heavily tested by fast/js/math.html."

I have reasonable confidence that this is covered by existing testing.  The larger question is what version of Mac OS X this was fixed in.  Presumably our bot coverage would answer that.

[Eric Seidel (no email)]

http://trac.webkit.org/browser/trunk/LayoutTests/fast/js/script-tests/math.js#L85

are relevant tests from fast/math.  I'm happy to add an additional one for ceil(-0.1).

Do you happen to know anything about this history of this bug?  I was landed w/o a radar or b.w.o reference.

[Darin Adler]

(In reply to comment #7)
> Do you happen to know anything about this history of this bug?  I was landed w/o a radar or b.w.o reference.

I found the bug, <rdar://problem/6286405>. Broken in Leopard, fixed in Snow Leopard.

[Eric Seidel (no email)]

I'm adding an explicit test for ceil(-0.1) just to be safe.  Will post an updated patch momentarily.

[Eric Seidel (no email)]

Created attachment 125970 [details]
Patch

[Eric Seidel (no email)]

I don't remember if Chrome still supports leopard or not.  webkit.org seems to have dropped official support for Leopard long ago.

[Eric Seidel (no email)]

I'm in no rush to land this patch.  But no sense in keeping this macro around if it's not needed. :)

[James Robinson]

Chromium still supports Leopard.

[Eric Seidel (no email)]

I will update this patch to make it leopard-only.

[Eric Seidel (no email)]

I will update this patch to make it leopard-only.

[Eric Seidel (no email)]

Created attachment 129113 [details]
Patch

[Eric Seidel (no email)]

/Volumes/Data/WebKit/Source/WebCore/platform/graphics/FractionalLayoutPoint.h: In function 'WebCore::IntPoint WebCore::ceiledIntPoint(const WebCore::FractionalLayoutPoint&)':
/Volumes/Data/WebKit/Source/WebCore/platform/graphics/FractionalLayoutPoint.h:157: error: 'class WebCore::FractionalLayoutUnit' has no member named 'ceil'
/Volumes/Data/WebKit/Source/WebCore/platform/graphics/FractionalLayoutPoint.h:157: error: 'class WebCore::FractionalLayoutUnit' has no member named 'ceil'

is the EWS error.  I'll have to update the patch a little.

[Adam Barth]

Comment on attachment 129113 [details]
Patch

This patch need to be updated because we changed the targeting macros.

[Ahmad Saleem]

Can we rename this - https://github.com/WebKit/WebKit/blob/609dbb726fceaaf552c60f8cc23219e78372c08f/Source/WebCore/platform/Decimal.cpp#L624

and 

https://searchfox.org/wubkat/source/Source/WebCore/html/InputType.cpp#1194

Similar to https://chromium.googlesource.com/chromium/blink/+/76937e6da246099dc3fb1cb1fcb9ebc2d73014f3

[Radar WebKit Bug Importer]

<rdar://problem/109645652>

[Anne van Kesteren]

Pull request: https://github.com/WebKit/WebKit/pull/14170

[EWS]

Committed 264323@main (bdf217f5a3d3): <https://commits.webkit.org/264323@main>

Reviewed commits have been landed. Closing PR #14170 and removing active labels.