Bug 124454 – [Win] WebKit version in user agent string is incorrect.

• Component: WebKit Misc.
• Status: RESOLVED FIXED

The WebKit version in the user agent string is outdated, it says "534+".

Created attachment 217125 [details]
Patch

[Brent Fulgham]

Comment on attachment 217125 [details]
Patch

Nice!  r=me.

[WebKit Commit Bot]

Comment on attachment 217125 [details]
Patch

Clearing flags on attachment: 217125

Committed r159430: <http://trac.webkit.org/changeset/159430>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

(In reply to comment #2)
> (From update of attachment 217125 [details])
> Nice!  r=me.

Thanks for reviewing :)

[WebKit Commit Bot]

Re-opened since this is blocked by bug 124548

[Tim Horton]

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.

Created attachment 217466 [details]
Patch

(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 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);
}

Created attachment 217826 [details]
Patch

(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 on attachment 217826 [details]
Patch

Attachment 217826 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/35368558

Created attachment 217834 [details]
Patch

[Brent Fulgham]

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 on attachment 217834 [details]
Patch

Clearing flags on attachment: 217834

Committed r159768: <http://trac.webkit.org/changeset/159768>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.