Bug 124454

Summary: [Win] WebKit version in user agent string is incorrect.
Product: WebKit Reporter: peavo
Component: WebKit Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, mrowe, thorton
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Unspecified   
Bug Depends on: 124548    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

peavo
Reported Saturday, November 16, 2013 4:31:06 PM UTC
The WebKit version in the user agent string is outdated, it says "534+".
Attachments
Patch (3.21 KB, patch)
2013-11-16 08:35 PST, peavo
no flags
Patch (3.05 KB, patch)
2013-11-20 12:39 PST, peavo
no flags
Patch (5.21 KB, patch)
2013-11-25 13:33 PST, peavo
no flags
Patch (7.47 KB, patch)
2013-11-25 14:41 PST, peavo
no flags
peavo
Comment 1 Saturday, November 16, 2013 4:35:46 PM UTC
Brent Fulgham
Comment 2 Monday, November 18, 2013 6:20:02 PM UTC
Comment on attachment 217125 [details] Patch Nice! r=me.
WebKit Commit Bot
Comment 3 Monday, November 18, 2013 6:23:15 PM UTC
Comment on attachment 217125 [details] Patch Clearing flags on attachment: 217125 Committed r159430: <http://trac.webkit.org/changeset/159430>
WebKit Commit Bot
Comment 4 Monday, November 18, 2013 6:23:16 PM UTC
All reviewed patches have been landed. Closing bug.
peavo
Comment 5 Monday, November 18, 2013 7:00:53 PM UTC
(In reply to comment #2) > (From update of attachment 217125 [details]) > Nice! r=me. Thanks for reviewing :)
WebKit Commit Bot
Comment 6 Tuesday, November 19, 2013 12:51:38 AM UTC
Re-opened since this is blocked by bug 124548
Tim Horton
Comment 7 Tuesday, November 19, 2013 12:56:45 AM UTC
If you want to do this, you'll have to do it in a way that doesn't involve reaching across the project boundary for random files.
peavo
Comment 8 Wednesday, November 20, 2013 8:39:24 PM UTC
peavo
Comment 9 Wednesday, November 20, 2013 8:41:29 PM UTC
(In reply to comment #7) > If you want to do this, you'll have to do it in a way that doesn't involve reaching across the project boundary for random files. Trying again :) I moved everything into the WebKit folder, and did not alter the user agent for WinApple, only WinCairo, to be on the safe side.
Brent Fulgham
Comment 10 Thursday, November 21, 2013 6:48:33 PM UTC
Comment on attachment 217466 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217466&action=review A really great idea. I think it needs a little bit of refinement to be right. I'd also like the Apple version to use your logic. > Source/WebKit/ChangeLog:8 > + * WebKit.vcxproj/WebKit/WebKitPreBuild.cmd: Generate WebKitVersion.h ... from OS X Version.xcconfig input file. > Source/WebKit/WebKit.vcxproj/WebKit/WebKitPreBuild.cmd:11 > bash "%WEBKIT_LIBRARIES%\tools\scripts\auto-version.sh" "%INTDIR%" auto-version.sh is supposed to do something clever about version, but I see that its input file is set to 534, rather than 538 (which is what we have in Version.xcconfig). I much prefer your approach, which allows us to control version in one place and keep them in sync. > Source/WebKit/WebKit.vcxproj/WebKit/WebKitPreBuild.cmd:17 > +if not exist "%WEBKITVERSIONFILE%" bash -c 'perl "%WEBKITVERSIONSCRIPT%" --config "%WEBKITVERSIONCONFIG%" --outputDir "%WEBKITVERSIONDIR%"' You need to make sure that %WEBKITVERSIONFILE% is deleted when you clean the project, or else it will never update when the value in Version.xcconfig changes. We should ideally drive generation of the version file based on changes to Version.xcconfig, but I'm not sure how we would do so in Visual Studio. We could add Version.xcconfig to the WebKit solution with a custom build step that would do this work, rather than having it done as part of WebKitPreBuild.cmd. Either way, this is wrong because you will get one version number from Version.xcconfig, and then it will never change again unless you delete the file. > Source/WebKit/win/WebView.cpp:2500 > +#endif Please revise as follows: #if !USE(CAIRO) LPWSTR buildNumberStringPtr; if (::LoadStringW(gInstance....) return buildNumberStringPtr; #endif return String::format("%d.%d", WEBKIT_MAJOR_VERSION, WEBKIT_MINOR_VERSION); }
peavo
Comment 11 Monday, November 25, 2013 9:33:58 PM UTC
peavo
Comment 12 Monday, November 25, 2013 9:36:06 PM UTC
(In reply to comment #10) > (From update of attachment 217466 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=217466&action=review > Thanks for looking into this, I added a custom build step to build WebKitVersion.h as you suggested :)
Build Bot
Comment 13 Monday, November 25, 2013 10:15:00 PM UTC
peavo
Comment 14 Monday, November 25, 2013 10:41:39 PM UTC
Brent Fulgham
Comment 15 Monday, November 25, 2013 11:02:32 PM UTC
Comment on attachment 217834 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217834&action=review Looks great! You might want to omit the macro as I describe below. But it's fine as-is. R=me. > Source/WebKit/win/WebView.cpp:2492 > +#if !USE(CAIRO) I'm not sure that we need this macro at all. If WinCairo doesn't define BUILD_NUMBER it should just do the right thing. That way, you can define it yourself for future builds.
WebKit Commit Bot
Comment 16 Monday, November 25, 2013 11:06:16 PM UTC
Comment on attachment 217834 [details] Patch Clearing flags on attachment: 217834 Committed r159768: <http://trac.webkit.org/changeset/159768>
WebKit Commit Bot
Comment 17 Monday, November 25, 2013 11:06:19 PM UTC
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.