Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#2781 closed task (fixed)

Review Robert Hogan's WebKit patches

Reported by: mikeperry Owned by: mikeperry
Priority: Medium Milestone:
Component: Applications/Torbutton Version:
Severity: Keywords: MikePerryIteration20110403 TorbuttonIteration20110403
Cc: Actual Points: 3
Parent ID: Points: 3
Reviewer: Sponsor:

Description

Robert Hogan has some patches he's working on for webkit. I need to review them, comment on them, and ask the Chrome folks about them.

Child Tickets

Change History (6)

comment:2 Changed 9 years ago by mikeperry

Ok, I guess I'll write up my thoughts here. I'm unfamiliar with webkit, so I'm first going to state my assumptions of what I think the code is trying to do.

First off, it looks like the primary use of these APIs is for C++ front ends implementing the ChromeClient interface, right? So each implementation would have to re-implement the QWebUserEnvironment interface in order to get their versions called by ChromeClient?

What was the discussion about Private APIs about? What is the difference between public and private APIs? Are private APIs only available to C++?

Does WebKit also provide window.screen.*? In Firefox, those values mirror a lot of the top level values here, including desktop resolution.

Also, I do not see you doing anything about screenX, screenY, and page[XY]Offset. You only appear to touch scrollX and Y? Is there a reason for this?

comment:3 in reply to:  2 Changed 9 years ago by mwenge

Replying to mikeperry:

First off, it looks like the primary use of these APIs is for C++ front ends implementing the ChromeClient interface, right?

Yes, and they could delegate it further, e.g. to extensions. This is the only way I can think of that allows a client, or a component in the client, to make decisions about DOM objects *and* CSS. I think the first two patches stand on their own in that regard. There may be better ways of delegating them to the client than I have come up with, but routing them to a part of WebKit that can at least talk to a WebKit browser is a good first step.

So each implementation would have to re-implement the QWebUserEnvironment interface in order to get their versions called by ChromeClient?

Yes. A Qt-based browser would reimplement QWebUserEnvironment. But exposing stuff to ChromeClient and FrameLoaderClient means other WebKit ports, including Chromium's, could also delegate them to their clients.

What was the discussion about Private APIs about? What is the difference between public and private APIs? Are private APIs only available to C++?

In this case, it's an API that's exported but not documented. There are a couple of Qt-style ways of doing this, but it would be a useful of way putting the API to use, getting a feel for whether it works, before committing to it in public API which is often very hard to roll back from it's wrong.

Does WebKit also provide window.screen.*? In Firefox, those values mirror a lot of the top level values here, including desktop resolution.

They're the same object in WebKit.

Also, I do not see you doing anything about screenX, screenY, and page[XY]Offset. You only appear to touch scrollX and Y? Is there a reason for this?

Yes, they're already sourced from ChromeClient, so can be delegated from there to a public API.

comment:4 Changed 9 years ago by mikeperry

Ok, this sounds great, then. I have one more question about the patch, and some other general questions that I might as well write here.

First, what about bit depth? window.screen has pixelDepth and colorDepth.

Also, do you happen to know if the new WebGL stuff gets this same information? It looks like WebGL also has a way of telling you things about the display: https://www.khronos.org/registry/webgl/specs/1.0/#5.2 (and 5.11).

I suspect it may be different, though, because it probably comes directly from OpenGL :/

Finally, there appears to be new fingerprinting bits in the DOM Range stuff for HTML5:

https://developer.mozilla.org/en/dom:range
https://developer.mozilla.org/en/DOM/element.getBoundingClientRect

It looks like you can get the rendered pixel size of arbitrary content elements, which is a fingerprint source.. Perhaps these should be added to your webkit Fingerprinting page. Probably beyond the scope of this patch.

comment:5 Changed 9 years ago by mikeperry

Actual Points: 3
Resolution: fixed
Status: newclosed

comment:6 Changed 9 years ago by mikeperry

In retrospect Robert, I don't think the bounding rectangle is something we can expect to be able to worry about. If rendering size changes per client, we need to address the root causes (like the font set provided, or the number of fonts available to a given page) rather than trying to mess with rendering.

I think we need to make the same call with WebGL. Pixel depth and other video card features is just something we're going to have to eat, I think. We should only concern ourselves with identifying information that is immaterial to rendering, such as desktop and widget size information.

Note: See TracTickets for help on using tickets.