Bug 51100

Summary: [QT] QtWebkit geolocation's position.timestamp is not in miliseconds
Product: WebKit Reporter: Mahesh Kulkarni <maheshk>
Component: WebCore Misc.Assignee: Mahesh Kulkarni <maheshk>
Status: RESOLVED FIXED    
Severity: Normal CC: ademar, bulach, commit-queue, hausmann, jarred, laszlo.gombos, mitz, steveblock
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
patch
kling: review-
patch none

Mahesh Kulkarni
Reported 2010-12-15 02:46:09 PST
According to HTML5 GeoLocation standards, (Geo)Position interface’s timestamp should be of DOMTimestamp type and DOMTimestamp represents a number of milliseconds. Ref: http://dev.w3.org/geo/api/spec-source.html#position_interface http://www.w3.org/TR/DOM-Level-3-Core/core.html#Core-DOMTimeStamp However in QTWebKit code while creating Geoposition object, timestamp was in ‘seconds ‘ (using QDateTime::toTime_t() function which returns the datetime as the number of seconds that have passed since 1970-01-01T00:00:00 UTC ) instead of ‘miliseconds’ (QDateTime:: toMSecsSinceEpoch() which returns the datetime as the number of milliseconds that have passed since 1970-01-01T00:00:00.000). So patched GeolocationServiceQt::positionUpdated() function to create Geoposition object using milliseconds.
Attachments
patch (1.87 KB, patch)
2010-12-20 00:21 PST, Mahesh Kulkarni 		kling: review-
DetailsFormatted DiffDiff
patch (2.10 KB, patch)
2010-12-20 05:40 PST, Mahesh Kulkarni 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Jarred Nicholls
Comment 1 2010-12-15 06:22:40 PST
diff --git a/WebCore/platform/qt/GeolocationServiceQt.cpp b/WebCore/platform/qt/GeolocationServiceQt.cpp index 3562eb9..2cccee6 100644 --- a/WebCore/platform/qt/GeolocationServiceQt.cpp +++ b/WebCore/platform/qt/GeolocationServiceQt.cpp @@ -83,7 +83,7 @@ void GeolocationServiceQt::positionUpdated(const QGeoPositionInfo &geoPosition) RefPtr<Coordinates> coordinates = Coordinates::create(latitude, longitude, providesAltitude, altitude, accuracy, providesAltitudeAccuracy, altitudeAccuracy, providesHeading, heading, providesSpeed, speed); - m_lastPosition = Geoposition::create(coordinates.release(), geoPosition.timestamp().toTime_t()); + m_lastPosition = Geoposition::create(coordinates.release(), geoPosition.timestamp().toMSecsSinceEpoch()); positionChanged(); } LayoutTests/fast/dom/Geolocation/timestamp.html appears to pass. Nice catch.
Mahesh Kulkarni
Comment 2 2010-12-20 00:21:21 PST
Created attachment 76979 [details] patch Proposing patch, now qtwebkit milisec timestamp from GeoPosition. Please review
Andreas Kling
Comment 3 2010-12-20 00:45:28 PST
Comment on attachment 76979 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=76979&action=review > WebCore/platform/qt/GeolocationServiceQt.cpp:86 > - m_lastPosition = Geoposition::create(coordinates.release(), geoPosition.timestamp().toTime_t()); > + m_lastPosition = Geoposition::create(coordinates.release(), geoPosition.timestamp().toMSecsSinceEpoch()); toMSecsSinceEpoch() is new in Qt 4.7. Since we still support building QtWebKit against Qt 4.6, this needs a QT_VERSION check and something for 4.6.
Mahesh Kulkarni
Comment 4 2010-12-20 05:40:28 PST
Created attachment 76991 [details] patch Thanks Kling for review. Incorporated review comments as per comment 3.
Andreas Kling
Comment 5 2010-12-20 06:27:58 PST
Comment on attachment 76991 [details] patch LGTM.
WebKit Commit Bot
Comment 6 2010-12-20 07:34:03 PST
Comment on attachment 76991 [details] patch Rejecting attachment 76991 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-attachment', '--non-interactive', 76991]" exit_code: 2 Last 500 characters of output: /ChangeLog Failed to merge in the changes. Patch failed at 0001 2010-12-20 Yury Semikhatsky <yurys@chromium.org> When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 132. Failed to run "['Tools/Scripts/update-webkit']" exit_code: 2 Full output: http://queues.webkit.org/results/7269066
WebKit Commit Bot
Comment 7 2010-12-20 08:23:51 PST
The commit-queue encountered the following flaky tests while processing attachment 76991 [details]: inspector/elements-panel-rewrite-href.html bug 51337 (authors: apavlov@chromium.org, pfeldman@chromium.org, and yurys@chromium.org) The commit-queue is continuing to process your patch.
Eric Seidel (no email)
Comment 8 2010-12-20 23:05:59 PST
Comment on attachment 76991 [details] patch That cq bug has finally been fixed. Sorry for the trouble.
WebKit Commit Bot
Comment 9 2010-12-20 23:49:04 PST
The commit-queue encountered the following flaky tests while processing attachment 76991 [details]: inspector/styles-disable-then-enable.html bug 51384 (author: pfeldman@chromium.org) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 10 2010-12-20 23:50:28 PST
Comment on attachment 76991 [details] patch Clearing flags on attachment: 76991 Committed r74404: <http://trac.webkit.org/changeset/74404>
WebKit Commit Bot
Comment 11 2010-12-20 23:50:34 PST
All reviewed patches have been landed. Closing bug.
Ademar Reis
Comment 12 2011-01-03 05:32:15 PST
Revision r74404 cherry-picked into qtwebkit-2.2 with commit be21b13 <http://gitorious.org/webkit/qtwebkit/commit/be21b13>
Note You need to log in before you can comment on or make changes to this bug.