Bug 88358 - Add navigator.buildType to indiciate the type of build (e.g., nightly, beta, final)
Summary: Add navigator.buildType to indiciate the type of build (e.g., nightly, beta, ...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Annie Sullivan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-05 13:05 PDT by Annie Sullivan
Modified: 2012-06-06 13:46 PDT (History)
8 users (show)

See Also:


Attachments
Patch (11.15 KB, patch)
2012-06-05 13:11 PDT, Annie Sullivan
no flags Details | Formatted Diff | Diff
Patch (6.39 KB, patch)
2012-06-06 13:45 PDT, Annie Sullivan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Annie Sullivan 2012-06-05 13:05:21 PDT
Add a string for the release channel, e.g. 'canary', 'dev', 'beta', 'stable'.
Comment 1 Annie Sullivan 2012-06-05 13:11:10 PDT
Created attachment 145858 [details]
Patch
Comment 2 Annie Sullivan 2012-06-05 13:15:28 PDT
(In reply to comment #1)
> Created an attachment (id=145858) [details]
> Patch

Work in progress. A few questions:

1) I can easily make a layout test that checks the value of navigator.channel, but the value will vary depending on how Chrome is built. Is there something better I can do than making expectations files which pass if it's defined as a string in chrome and if it's not defined in the other ports?

2) Is the approach of pulling in the release branch from the GYP files okay? The other approach I can use is calling chrome::VersionInfo::Channel() at runtime.

3) For local builds and anything else that's not an official channel, what should navigator.channel read? "unknown" like in the channel enum?
Comment 3 mitz 2012-06-05 13:41:32 PDT
This doesn’t seem like something that should be part of WebKit. A client application should be able to use WebKit API to add this property. The right place to do this would be in the client’s didClearWindowObjectForFrame() implementation.
Comment 4 Adam Barth 2012-06-05 14:38:54 PDT
> This doesn’t seem like something that should be part of WebKit. A client application should be able to use WebKit API to add this property.

We actually had an implementation in the client, but I asked Annie to write a patch for WebKit so that this property can live along with appCodeName and the other similar properties on Navigator.

Is there a particular reason why this isn't appropriate for WebKit (as compared wit the other similar existing navigator properties)?
Comment 5 Alexey Proskuryakov 2012-06-05 14:49:11 PDT
I don't think that there is anything specific to the property at hand, just the same reason why we are not adding functionality unique to Apple clients (like iTunes) to WebCore.

While technically it's of course easier to have related properties together, I agree with Mitz that client functionality should stay out of WebCore as much as technically possible.
Comment 6 Adam Barth 2012-06-05 15:02:10 PDT
Your comment presupposes that this is client functionality.  Why is it any more a client functionality than appCodeName?
Comment 7 Alexey Proskuryakov 2012-06-05 15:21:32 PDT
I may be not following your line of thought, because it feels like a very clear distinction to me.

Doing what every browser needs is part of WebKit project, but navigator.channel is unique to one browser based on WebKit, and is entirely meaningless to every non-chromium person working on the project.
Comment 8 mitz 2012-06-05 15:28:44 PDT
Thanks, Alexey. I couldn’t have said it better.
Comment 9 Adam Barth 2012-06-05 15:41:21 PDT
> Doing what every browser needs is part of WebKit project, but navigator.channel is unique to one browser based on WebKit, and is entirely meaningless to every non-chromium person working on the project.

Is that just a naming issue?  Many consumers of WebKit have notions of a "beta" release of their product.  The notion of a beta release is hardly Chromium-specific.  For example, Safari 3 had a beta release.

From an interface perspective, it seems quite analogous to appVersion.
Comment 10 Alexey Proskuryakov 2012-06-05 16:33:34 PDT
> The notion of a beta release is hardly Chromium-specific.

We have not exposed that via DOM though, and even if we did, using the name "channel" is far from a given.

Yes, I agree that it's analogous - alluded to that in comment 5 too. From the perspective of a person who works on a project that exposes both navigator.appName and navigator.channel implementing them together makes a lot of sense. This is also completely alien and weird for others. In WebKit project, we've been working fairly hard on separating areas of concern, and keeping navigator.channel away from view of people who never need to hear about it just feels natural (as long as technical costs don't become exorbitant).
Comment 11 Adam Barth 2012-06-05 16:43:57 PDT
Do you have a suggestion for a name other than "channel"?  I don't think we have any attachment to that name.

I don't understand what concerns you're trying to separate.  The navigator object is a web platform API for exposing information about the user agent, such as the user agent string, it's version, etc.
Comment 12 Alexey Proskuryakov 2012-06-05 17:28:00 PDT
I don't have any better name than "channel" to expose which Chrome channel the browser came from, except for perhaps "chromeChannel". Other browsers don't come from Chrome channels, and don't expose any similar data, so renaming cannot help.

I think that the existing solution with injecting the property from client code is best.
Comment 13 Adam Barth 2012-06-05 17:38:26 PDT
I I don't think chromeChannel is an appropriate name because this concept is not specific to Chrome.

Perhaps buildType would be a better name.  Firefox seems to call refer to their "beta build" and to Aurora as a "pre-beta" build.  nightly.webkit.org refers to "nightly builds" and might reasonably return the string "nightly" from this API.
Comment 14 Adam Barth 2012-06-05 17:39:54 PDT
@ap: I don't agree that the builtType is a Chrome-specific concept.  In fact, it seems to be a concept used actively by every browser (with the possible exception of Safari---which used the concept as recently as Safari 3 beta).

@Annie: Would you be willing to update the patch to use the name "buildType" ?  Also, rather than implementing this as a Module, it's probably better to just define the attribute in Navigator.idl and use an ENABLE macro.
Comment 15 Alexey Proskuryakov 2012-06-05 22:55:26 PDT
> I don't agree that the builtType is a Chrome-specific concept.  In fact, it seems to be a concept used actively by every browser

Well, I guess you need to get support from other vendors of WebKit-based products on webkit-dev if you want this in WebCore.
Comment 16 Adam Barth 2012-06-06 00:17:24 PDT
Indeed.  Annie, would you be willing to email webkit-dev as described in http://www.webkit.org/coding/adding-features.html ?
Comment 17 Annie Sullivan 2012-06-06 13:45:37 PDT
Created attachment 146101 [details]
Patch
Comment 18 Annie Sullivan 2012-06-06 13:46:56 PDT
(In reply to comment #14)
> @Annie: Would you be willing to update the patch to use the name "buildType" ?  Also, rather than implementing this as a Module, it's probably better to just define the attribute in Navigator.idl and use an ENABLE macro.

I updated the patch.(In reply to comment #16)
> Indeed.  Annie, would you be willing to email webkit-dev as described in http://www.webkit.org/coding/adding-features.html ?

Done.