Re: [webkit-dev] Adding window.layoutTestInspector
On a related note, there are a few features such as geolocation, device orientation and speech which require input from hardware devices. To test these using layout tests, we are considering passing mock objects to WebCore and the mocks would either give fake device data or just verify the order/frequency of calls made by WebCore. There are 2 open bugs with sample patches for these available now, https://bugs.webkit.org/show_bug.cgi?id=42603 and https://bugs.webkit.org/show_bug.cgi?id=39589, I'm interested to hear if anyone has comments on these approaches. Having the base LayoutTestController defined in WebCore would simplify testing such features as we can add such feature-specific test methods without having to add test-specific plumbing in WebKit. Cheers Satish On Wed, Jul 21, 2010 at 6:33 AM, Hajime Morita morr...@google.com wrote: Hi Maciej, thanks much for sharing your thought. Overall, it totally makes sense. And we need an action as Ojan mentioned. Here is some ideas; (1) It seems a little odd that we'll end up with two different objects that have similar names and a very similar purpose, but just differ in how they are implemented. Maybe there's a way to define layoutTestController in WebCore and have DumpRenderTree extend it. It looks fine. A prototype patch will come shortly. (2) It does seem like for some test-specific methods, implementing then in WebCore would be simpler and would save the work of plumbing them through the WebKit layers. Yeah, it's the main point of this proposal! (3) On the other hand, LayoutTestController seems like it has too much stuff in it. Originally DumpRenderTree exposed a very modest set of functionality, mostly to control output (dumpAsText) or to emulate things that you could do by hand when running the test in the browser (waitUntilDone, eventSender). Nowadays, there are dozens of methods. A lot of them are used in only one or two tests. And in many cases, the methods have no interactive equivalent, so a lot of our tests are not runnable in the browser at all. Those seem like bad trends. Maybe instead of making it easier to add to LayoutTestController, we should look at whether we can consolidate functionality, factor it into more objects, and find ways to test things that don't require quite so much custom functionality. Looks several points here and I think we can tackle them separetely: - (3-1): LayoutTestController is too large and need to be factored out - (3-2): We shoud allow our tests to run interactively in the browser. For (3-1), once we can implement test objects inside WebCore, it will become easier to do so because we can use our internal mechanism like IDL-based binding generation. As an example, defining WebSettings JS object to expose Settings parameter would be fine. 16 of 120 LayoutTestController APIs are Settings/WebPreferences setters. WebView and WebFrame also have several wrapper APIs on LayoutTestController. Tackling (3-2) looks harder than for (3-1). As WebKit grows, no single browser cannot (and doesn't need to) access all of WebKit features. Each of them uses different subset of features. Browsers themselves also behave differently. There are over 20 flags on LayoutTestController to control its behavior, which implies how our browsers collaborate WebKit in many different ways. So problem (3-2) is not always of LayoutTestController. It may just reflect the fact of, say, divergence of our codebase and use-cases. On the other hand, I agree that we need to run our tests interactively. One possible idea is: Adding a switch to the inspector that make LayoutTestController (and associated objects) accessible from the page context. Once we have LayoutTestController inside WebCore, these objects will be available without DRT. It might not solve the root problem. But it would provide the way to workround. -- morita On Tue, Jul 20, 2010 at 2:51 PM, Maciej Stachowiak m...@apple.com wrote: Darin and I discussed this proposal, and we had a few thoughts to share: (1) It seems a little odd that we'll end up with two different objects that have similar names and a very similar purpose, but just differ in how they are implemented. Maybe there's a way to define layoutTestController in WebCore and have DumpRenderTree extend it. (2) It does seem like for some test-specific methods, implementing then in WebCore would be simpler and would save the work of plumbing them through the WebKit layers. (3) On the other hand, LayoutTestController seems like it has too much stuff in it. Originally DumpRenderTree exposed a very modest set of functionality, mostly to control output (dumpAsText) or to emulate things that you could do by hand when running the test in the browser (waitUntilDone, eventSender). Nowadays, there are dozens of methods. A lot of them are used in only one or two tests. And in many cases, the methods have no interactive equivalent,
[webkit-dev] Clients and the Page constructor
Currently, nine clients are passed to the Page constructor and the number is growing. Recently, clients have been added for Geolocation, DeviceOrientation and BackForwardController. This approach doesn't seem scalable. Instead, I'd like to suggest that clients, at least those for optional features, are not passed to the Page constructor. Instead, the client should default to null and can be set with an explicit method call, eg page-setFooClient(client) or page-getFooController()-setClient(client). This is the approach that was taken for SpeechInput in http://trac.webkit.org/changeset/63230. If there are no objections, I'll send patches to make this change for Geolocation and DeviceOrientation. For reference, see the discussion in https://bugs.webkit.org/show_bug.cgi?id=39589. Thanks, Steve ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Clients and the Page constructor
On Jul 21, 2010, at 8:47 AM, Steve Block wrote: Currently, nine clients are passed to the Page constructor and the number is growing. Recently, clients have been added for Geolocation, DeviceOrientation and BackForwardController. This approach doesn't seem scalable. What exactly does not scalable mean? Passing arguments to the constructor rather than setting them up later is often good because there is no time window where the object is not set up. Generally speaking we don’t want to have to write code to handle a client of 0 unless it’s absolutely necessary. Instead, the client should default to null and can be set with an explicit method call Another option, if the objection is a function with a lot of arguments, is to pass in a structure with all the client pointers to the constructor. What concrete problem are we trying to solve here? The words “doesn’t seem scalable” alone don’t make that clear to me. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] svg/filters/filter-empty-g.svg ASSERTs on Leopard Intel Debug (Tests)
On Mon, Jul 19, 2010 at 2:04 AM, Adam Barth aba...@webkit.org wrote: On Sun, Jul 18, 2010 at 2:06 PM, Nikolas Zimmermann zimmerm...@physik.rwth-aachen.de wrote: Am 18.07.2010 um 18:36 schrieb Adam Barth: I'm not sure it's working properly. It says: SUCCESS: Build 17401 (r63531) was the first to show failures: set([u'svg/filters/filter-empty-g.svg']) but then it goes on to list all the commits from r63492 to r63531. I already notified the orginal author at https://bugs.webkit.org/show_bug.cgi?id=41175 that something is broken -- if nothing happens please can someone revert it, I'm not here until Tuesday. Thanks. Hopefully we'll be able to resolve the issue before midday Monday. This test is still crashing on the bots. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Clients and the Page constructor
What exactly does not scalable mean? Passing arguments to the constructor rather than setting them up later is often good because there is no time window where the object is not set up. Generally speaking we don’t want to have to write code to handle a client of 0 unless it’s absolutely necessary. Agreed. My motivation was to avoid the long, growing, list of arguments and to avoid the need to update the call sites for all platforms with an extra '0' each time a new optional feature is added. Another option, if the objection is a function with a lot of arguments, is to pass in a structure with all the client pointers to the constructor. That sounds reasonable. A structure of the client pointers for all optional features, with each entry defaulting to 0, would solve this problem. Another argument for the setter is that it makes it easier to inject a mock for testing in response to a LayoutTestController method call, by simply calling the setter again to replace the real client with a mock client. Personally, however, I think that this is somewhat abusing the idea of setting a client. If the real implementation needs to be switched for a mock at runtime in DRT, this should probably be handled by the client itself - so the same client is used throughout the lifetime of the Page. Steve ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Clients and the Page constructor
That still makes it required to add null checks for some (or all) of the clients. Something we've avoided in the past. This wouldn't add a need for null checks. The structure would only be for clients for optional features. If the feature is enabled, the client must be non-null. This is no different from the case with the individual arguments used currently. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] review queue crazy idea
There are currently 38 (of 171 total) patches in the review queue where the bugs have not been modified in over 1 month old. I propose we have a bot that educates people about writing easy to review patches and auto-rejects any patches in bugs that haven't been touched in over a month. For people new-ish to the WebKit project, it is often confusing both degree of responsibility that lies with the contributor to make the patch easy to review and the need to get reviewers' attention for a given patch. This is just an initial proposal. I'm not wed to any of the details of how this would work. I do think that auto-rejecting old patches is valuable to the project as a whole. Having the review queue be so large makes it daunting for any reviewer to try and tackle it. On the other hand, knowing that patches will magically fall off the end of the queue might encourage reviewers to just ignore some patches. An alternative to auto-rejecting patches would be to send a nag email once a week to webkit-reviewers@ with the list of patches that are over a month old. Here are my initial thoughts on what a review bot would do. *After a patch turns a week old, send the following email:* Patch 12345 of bug 6789 is a week old. It may just be because no reviewer has found time to review it. But there may be steps you can take to help get your patch reviewed. See http://trac.webkit.org/wiki/CodeReview for a few suggestions. -WebKit review bot *After the patch is three weeks old:* Patch 12345 of bug 6789 is three weeks old. If it is still unreviewed in a week, it will automatically be rejected. It may just be because no reviewer has found time to review it. But there may be steps you can take to help get your patch reviewed. See http://trac.webkit.org/wiki/CodeReview for a few suggestions. -WebKit review bot *After the patch is a month old:* Patch 12345 of bug 6789 has been rejected because it is too old. This is likely because no webkit reviewer has been able to review it. If you would still like the patch reviewed, then please do the following: 1. Make sure your patch still applies to tip of tree. 2. Do as many of the suggestions at http://trac.webkit.org/wiki/CodeReview as possible. 3. Upload your patch for review again. -Webkit Review Bot ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] review queue crazy idea
On Wed, Jul 21, 2010 at 2:40 PM, Ojan Vafai o...@chromium.org wrote: Here are my initial thoughts on what a review bot would do. *After a patch turns a week old, send the following email:* Patch 12345 of bug 6789 is a week old. It may just be because no reviewer has found time to review it. But there may be steps you can take to help get your patch reviewed. See http://trac.webkit.org/wiki/CodeReview for a few suggestions. -WebKit review bot *After the patch is three weeks old:* Patch 12345 of bug 6789 is three weeks old. If it is still unreviewed in a week, it will automatically be rejected. It may just be because no reviewer has found time to review it. But there may be steps you can take to help get your patch reviewed. See http://trac.webkit.org/wiki/CodeReview for a few suggestions. -WebKit review bot *After the patch is a month old:* Patch 12345 of bug 6789 has been rejected because it is too old. This is likely because no webkit reviewer has been able to review it. If you would still like the patch reviewed, then please do the following: 1. Make sure your patch still applies to tip of tree. 2. Do as many of the suggestions at http://trac.webkit.org/wiki/CodeReview as possible. 3. Upload your patch for review again. If we want to implement this, I think there ought to be some sort of easy way for a patch author to respond to any of these automated actions with some kind of a I looked at the suggestions, my patch is as easy-to-review as possible, please don't close it and instead help me flag down some reviewers action. PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] review queue crazy idea
On Jul 21, 2010, at 2:40 PM, Ojan Vafai wrote: There are currently 38 (of 171 total) patches in the review queue where the bugs have not been modified in over 1 month old. I propose we have a bot that educates people about writing easy to review patches and auto-rejects any patches in bugs that haven't been touched in over a month. For people new-ish to the WebKit project, it is often confusing both degree of responsibility that lies with the contributor to make the patch easy to review and the need to get reviewers' attention for a given patch. This is just an initial proposal. I'm not wed to any of the details of how this would work. I do think that auto-rejecting old patches is valuable to the project as a whole. Having the review queue be so large makes it daunting for any reviewer to try and tackle it. On the other hand, knowing that patches will magically fall off the end of the queue might encourage reviewers to just ignore some patches. An alternative to auto-rejecting patches would be to send a nag email once a week to webkit-reviewers@ with the list of patches that are over a month old. I think we should try the nag email first. I like the idea of advice on how to get a review. I think automatic rejection is kind of unfriendly, so I'd like to try other steps first. - Maciej Here are my initial thoughts on what a review bot would do. After a patch turns a week old, send the following email: Patch 12345 of bug 6789 is a week old. It may just be because no reviewer has found time to review it. But there may be steps you can take to help get your patch reviewed. See http://trac.webkit.org/wiki/CodeReview for a few suggestions. -WebKit review bot After the patch is three weeks old: Patch 12345 of bug 6789 is three weeks old. If it is still unreviewed in a week, it will automatically be rejected. It may just be because no reviewer has found time to review it. But there may be steps you can take to help get your patch reviewed. See http://trac.webkit.org/wiki/CodeReview for a few suggestions. -WebKit review bot After the patch is a month old: Patch 12345 of bug 6789 has been rejected because it is too old. This is likely because no webkit reviewer has been able to review it. If you would still like the patch reviewed, then please do the following: Make sure your patch still applies to tip of tree. Do as many of the suggestions at http://trac.webkit.org/wiki/CodeReview as possible. Upload your patch for review again. -Webkit Review Bot ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] review queue crazy idea
Wow. I really like this idea of helping contributors better understand what's going wrong. But, I think that even better would be to build a better front-end for reviews. Or a bot which knew how to suggest reviewers (based on annotate information from lines changed). I encourage you to write any sort of better review tool/bot, turn it on, and see what happens. That's kinda what we did with the EWS and commit-queue. Initial reactions (to both) were strongly negative, but we fixed a bunch of stuff from initial feedback, and I (would like to) believe we have two useful systems now. I see the same pattern happening for someone trying to build some better review tools. -eric On Wed, Jul 21, 2010 at 5:40 PM, Ojan Vafai o...@chromium.org wrote: There are currently 38 (of 171 total) patches in the review queue where the bugs have not been modified in over 1 month old. I propose we have a bot that educates people about writing easy to review patches and auto-rejects any patches in bugs that haven't been touched in over a month. For people new-ish to the WebKit project, it is often confusing both degree of responsibility that lies with the contributor to make the patch easy to review and the need to get reviewers' attention for a given patch. This is just an initial proposal. I'm not wed to any of the details of how this would work. I do think that auto-rejecting old patches is valuable to the project as a whole. Having the review queue be so large makes it daunting for any reviewer to try and tackle it. On the other hand, knowing that patches will magically fall off the end of the queue might encourage reviewers to just ignore some patches. An alternative to auto-rejecting patches would be to send a nag email once a week to webkit-reviewers@ with the list of patches that are over a month old. Here are my initial thoughts on what a review bot would do. After a patch turns a week old, send the following email: Patch 12345 of bug 6789 is a week old. It may just be because no reviewer has found time to review it. But there may be steps you can take to help get your patch reviewed. See http://trac.webkit.org/wiki/CodeReview for a few suggestions. -WebKit review bot After the patch is three weeks old: Patch 12345 of bug 6789 is three weeks old. If it is still unreviewed in a week, it will automatically be rejected. It may just be because no reviewer has found time to review it. But there may be steps you can take to help get your patch reviewed. See http://trac.webkit.org/wiki/CodeReview for a few suggestions. -WebKit review bot After the patch is a month old: Patch 12345 of bug 6789 has been rejected because it is too old. This is likely because no webkit reviewer has been able to review it. If you would still like the patch reviewed, then please do the following: Make sure your patch still applies to tip of tree. Do as many of the suggestions at http://trac.webkit.org/wiki/CodeReview as possible. Upload your patch for review again. -Webkit Review Bot ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] review queue crazy idea
Hey, I just don't understand how can the patches can sit in bugzilla unreviewed for weeks? There are 76 reviewers in the trac's team list. I think the reviewers have to do what they have assumed... Reviewing patches! I agree with Maciej automatic rejection is unfriendly. (Of course we can reject patches which are no longer applies on trunk. Yes, we should do this!) I think we should find a good way to advise the reviewers of the patch's topic. I prefer this way of automation. Regards, Zoltan On Wednesday 21 July 2010, at 23:40, Ojan Vafai wrote: There are currently 38 (of 171 total) patches in the review queue where the bugs have not been modified in over 1 month old. I propose we have a bot that educates people about writing easy to review patches and auto-rejects any patches in bugs that haven't been touched in over a month. For people new-ish to the WebKit project, it is often confusing both degree of responsibility that lies with the contributor to make the patch easy to review and the need to get reviewers' attention for a given patch. This is just an initial proposal. I'm not wed to any of the details of how this would work. I do think that auto-rejecting old patches is valuable to the project as a whole. Having the review queue be so large makes it daunting for any reviewer to try and tackle it. On the other hand, knowing that patches will magically fall off the end of the queue might encourage reviewers to just ignore some patches. An alternative to auto-rejecting patches would be to send a nag email once a week to webkit-reviewers@ with the list of patches that are over a month old. Here are my initial thoughts on what a review bot would do. *After a patch turns a week old, send the following email:* Patch 12345 of bug 6789 is a week old. It may just be because no reviewer has found time to review it. But there may be steps you can take to help get your patch reviewed. See http://trac.webkit.org/wiki/CodeReview for a few suggestions. -WebKit review bot *After the patch is three weeks old:* Patch 12345 of bug 6789 is three weeks old. If it is still unreviewed in a week, it will automatically be rejected. It may just be because no reviewer has found time to review it. But there may be steps you can take to help get your patch reviewed. See http://trac.webkit.org/wiki/CodeReview for a few suggestions. -WebKit review bot *After the patch is a month old:* Patch 12345 of bug 6789 has been rejected because it is too old. This is likely because no webkit reviewer has been able to review it. If you would still like the patch reviewed, then please do the following: 1. Make sure your patch still applies to tip of tree. 2. Do as many of the suggestions at http://trac.webkit.org/wiki/CodeReview as possible. 3. Upload your patch for review again. -Webkit Review Bot ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] review queue crazy idea
Patches sit in the queue for numerous reasons. Some of us used to scan the queue every day. But I've stopped doing that. Now I scan it more like once a week or two. There is no good way to find which patches might I have a chance of reviewing, so you end up spending 30 minutes just to find a patch you could actually review. Most patches get rejected for easily-bot-detectable reasons. Like bad or missing ChangeLogs, poor comment style, tabs, breaking EWS bots. Now that the style bot and EWS bots work better we should at least cq- patches which fail those bots (or fail to apply). I think reviewing would be much easier if we had some better site by which to review. I'm not sure how we solve the social problem of getting people to review patches which didn't come from the person sitting next to them in their office however. Then again, that's sorta OK. part of contributing to webkit is integrating into the community. It's importnat to know your reviewers and to discuss things with them in channels other than the bug. But I still think some minimal technical work would go a long way to making reviewing better. -eric On Wed, Jul 21, 2010 at 7:34 PM, Zoltan Horvath zol...@webkit.org wrote: Hey, I just don't understand how can the patches can sit in bugzilla unreviewed for weeks? There are 76 reviewers in the trac's team list. I think the reviewers have to do what they have assumed... Reviewing patches! I agree with Maciej automatic rejection is unfriendly. (Of course we can reject patches which are no longer applies on trunk. Yes, we should do this!) I think we should find a good way to advise the reviewers of the patch's topic. I prefer this way of automation. Regards, Zoltan On Wednesday 21 July 2010, at 23:40, Ojan Vafai wrote: There are currently 38 (of 171 total) patches in the review queue where the bugs have not been modified in over 1 month old. I propose we have a bot that educates people about writing easy to review patches and auto-rejects any patches in bugs that haven't been touched in over a month. For people new-ish to the WebKit project, it is often confusing both degree of responsibility that lies with the contributor to make the patch easy to review and the need to get reviewers' attention for a given patch. This is just an initial proposal. I'm not wed to any of the details of how this would work. I do think that auto-rejecting old patches is valuable to the project as a whole. Having the review queue be so large makes it daunting for any reviewer to try and tackle it. On the other hand, knowing that patches will magically fall off the end of the queue might encourage reviewers to just ignore some patches. An alternative to auto-rejecting patches would be to send a nag email once a week to webkit-reviewers@ with the list of patches that are over a month old. Here are my initial thoughts on what a review bot would do. *After a patch turns a week old, send the following email:* Patch 12345 of bug 6789 is a week old. It may just be because no reviewer has found time to review it. But there may be steps you can take to help get your patch reviewed. See http://trac.webkit.org/wiki/CodeReview for a few suggestions. -WebKit review bot *After the patch is three weeks old:* Patch 12345 of bug 6789 is three weeks old. If it is still unreviewed in a week, it will automatically be rejected. It may just be because no reviewer has found time to review it. But there may be steps you can take to help get your patch reviewed. See http://trac.webkit.org/wiki/CodeReview for a few suggestions. -WebKit review bot *After the patch is a month old:* Patch 12345 of bug 6789 has been rejected because it is too old. This is likely because no webkit reviewer has been able to review it. If you would still like the patch reviewed, then please do the following: 1. Make sure your patch still applies to tip of tree. 2. Do as many of the suggestions at http://trac.webkit.org/wiki/CodeReview as possible. 3. Upload your patch for review again. -Webkit Review Bot ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] focusin/focusout events
WebKit doesn't match the DOM 3 spec WRT focusin/focusout events, see
http://dev.w3.org/2006/webapi/DOM-Level-3-Events/html/DOM3-Events.html#events-focusevent-doc-focus
. Specifically, focusin/focusout are supposed to fire before the element
actually receives/loses focus. Browsers are wildly inconsistent with
focus-related events, but only WebKit and IE support focusin/focusout and
only WebKit gets this bit wrong.
Also, there are a few things that you ought to be able to do that you can't
currently:
1. Capture and handle a blur-type event before focus is moved or the
selection is cleared/moved.
2. Capture and handle a focus-type event before focus is moved or the
selection is cleared/moved.
In WebKit, the selection and focus are cleared from the element being
blurred before any events are fired. That's lametastic. One common use-case
here is making keyboard-accessible UI widgets for a rich-text area. For
keyboard-accessible UI widgets, especially ones that require text input
(e.g. a font combo-box), you need to save the selection before blurring.
Instead of simply listening to a blur event, you need to instrument every
place that might clear the selection when clicked in order to save it first.
I've listed below the order events are fired in different browsers when one
element is blurred and another is focused.
I was using roughly this code:
div contentEditable id=fooasdf/div
div contentEditable id=barqwer/div
script
var foo = document.getElementById('foo');
var bar = document.getElementById('bar');
bar.addEventListener('DOMFocusIn', function() { console.log('focusin: ' +
document.activeElement.id) }, true);
bar.addEventListener('focus', function() { console.log('focus: ' +
document.activeElement.id) }, true);
foo.addEventListener('DOMFocusOut', function() { console.log('focusout: ' +
document.activeElement.id) }, true);
foo.addEventListener('blur', function() { console.log('blur: ' +
document.activeElement.id) }, true);
/script
Opera 10.6: blur, domfocusout, focus, domfocusin
Moves activeElement immediately before firing any events.
WebKit nightly: blur, (dom)focusout, focus, (dom)focusin
Clears activeElement before blur/focusout, sets active element before
focus/focusin
FF 3.6:blur, focus
Clears activeElement before blur, sets active element before focus.
IE 8: focusout, focusin, blur, focus
Moves activeElement immediately before firing any events.
It's a bit gross, but the only solution I can think of is to fire the
following events in WebKit (in this order):
focusout
focusin
{remove focus and clear selection as appropriate}
blur
domfocusout
{add focus to new node and set selection as appropriate}
focus
domfocusin
I'd like to say we could get rid of DOMFocusOut/DOMFocusIn or blur/focus,
but I expect too many sites would break.
What do you all think? I care more about meeting the use-case listed above
than matching the spec. If we can meet the above use-case differently, I
expect we could get the spec modified.
Ojan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Enabling the HTML5 tree builder soon
We're getting close to enabling the HTML5 tree builder on trunk. Once we do that, we'll have the core of the HTML5 parsing algorithm turned on, including SVG-in-HTML. There are still a bunch of details left to finish (such as fragment parsing, MathML entities, and better error reporting), but this marks a significant milestone for this work. The tree builder is markedly more complicated than the tokenizer, and I'm sure we're going to have some bad regressions. I'd like to ask your patience and your help to spot and triage these regressions. We've gotten about as much mileage as we can out of the HTML5lib test suite and the LayoutTests. The next step for is to see how the algorithm works in the real world. There are about 84 tests that will require new expectations, mostly due to invisible differences in render tree dumps (e.g., one more or fewer 0x0 render text). In about half the cases, we've manually verified that our new results agree with the Firefox nightly builds, which is great from a compliance and interoperability point of view. The other half involve things like the exact text for the isindex, which we've chosen to match the spec exactly, or the keygen element, which needs some shadow DOM love to hide its implementation details from web content. As for performance, last time we ran our parser benchmark, the new tree builder was 1% faster than the old tree builder. There's still a bunch of low-hanging performance work we can do, such as atomizing strings and inlining functions. If you're interested in performance, let me or Eric know and we can point you in the right direction. I don't have an exact timeline for when we're going to throw the switch, but sometime in the next few days. If you'd like us to hold off for any reason, please let Eric or me know. Adam P.S., you can follow along by CCing yourself on the master bug, https://bugs.webkit.org/show_bug.cgi?id=41123, or by looking at our LayoutTest failure triage spreadsheet, https://spreadsheets.google.com/ccc?key=0AlC4tS7Ao1fIdEo0SFdLaVpiclBHMVNQcHlTenV5TEEhl=en. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Adding window.layoutTestInspector
Hi folks, I posted a patch: https://bugs.webkit.org/show_bug.cgi?id=42612 Any feedbacks are appreciated. -- morita On Wed, Jul 21, 2010 at 2:33 PM, Hajime Morita morr...@google.com wrote: Hi Maciej, thanks much for sharing your thought. Overall, it totally makes sense. And we need an action as Ojan mentioned. Here is some ideas; (1) It seems a little odd that we'll end up with two different objects that have similar names and a very similar purpose, but just differ in how they are implemented. Maybe there's a way to define layoutTestController in WebCore and have DumpRenderTree extend it. It looks fine. A prototype patch will come shortly. (2) It does seem like for some test-specific methods, implementing then in WebCore would be simpler and would save the work of plumbing them through the WebKit layers. Yeah, it's the main point of this proposal! (3) On the other hand, LayoutTestController seems like it has too much stuff in it. Originally DumpRenderTree exposed a very modest set of functionality, mostly to control output (dumpAsText) or to emulate things that you could do by hand when running the test in the browser (waitUntilDone, eventSender). Nowadays, there are dozens of methods. A lot of them are used in only one or two tests. And in many cases, the methods have no interactive equivalent, so a lot of our tests are not runnable in the browser at all. Those seem like bad trends. Maybe instead of making it easier to add to LayoutTestController, we should look at whether we can consolidate functionality, factor it into more objects, and find ways to test things that don't require quite so much custom functionality. Looks several points here and I think we can tackle them separetely: - (3-1): LayoutTestController is too large and need to be factored out - (3-2): We shoud allow our tests to run interactively in the browser. For (3-1), once we can implement test objects inside WebCore, it will become easier to do so because we can use our internal mechanism like IDL-based binding generation. As an example, defining WebSettings JS object to expose Settings parameter would be fine. 16 of 120 LayoutTestController APIs are Settings/WebPreferences setters. WebView and WebFrame also have several wrapper APIs on LayoutTestController. Tackling (3-2) looks harder than for (3-1). As WebKit grows, no single browser cannot (and doesn't need to) access all of WebKit features. Each of them uses different subset of features. Browsers themselves also behave differently. There are over 20 flags on LayoutTestController to control its behavior, which implies how our browsers collaborate WebKit in many different ways. So problem (3-2) is not always of LayoutTestController. It may just reflect the fact of, say, divergence of our codebase and use-cases. On the other hand, I agree that we need to run our tests interactively. One possible idea is: Adding a switch to the inspector that make LayoutTestController (and associated objects) accessible from the page context. Once we have LayoutTestController inside WebCore, these objects will be available without DRT. It might not solve the root problem. But it would provide the way to workround. -- morita On Tue, Jul 20, 2010 at 2:51 PM, Maciej Stachowiak m...@apple.com wrote: Darin and I discussed this proposal, and we had a few thoughts to share: (1) It seems a little odd that we'll end up with two different objects that have similar names and a very similar purpose, but just differ in how they are implemented. Maybe there's a way to define layoutTestController in WebCore and have DumpRenderTree extend it. (2) It does seem like for some test-specific methods, implementing then in WebCore would be simpler and would save the work of plumbing them through the WebKit layers. (3) On the other hand, LayoutTestController seems like it has too much stuff in it. Originally DumpRenderTree exposed a very modest set of functionality, mostly to control output (dumpAsText) or to emulate things that you could do by hand when running the test in the browser (waitUntilDone, eventSender). Nowadays, there are dozens of methods. A lot of them are used in only one or two tests. And in many cases, the methods have no interactive equivalent, so a lot of our tests are not runnable in the browser at all. Those seem like bad trends. Maybe instead of making it easier to add to LayoutTestController, we should look at whether we can consolidate functionality, factor it into more objects, and find ways to test things that don't require quite so much custom functionality. I'll add on my own behalf that layoutTestInspector doesn't seem like a great name and doesn't express the relationship to layoutTestController. It's not used to examine layout tests. Regards, Maciej On Jul 14, 2010, at 10:16 PM, Hajime Morita wrote: Hi WebKit folks, I'm planning to add
Re: [webkit-dev] review queue crazy idea
On Wed, Jul 21, 2010 at 2:40 PM, Ojan Vafai o...@chromium.org wrote: There are currently 38 (of 171 total) patches in the review queue where the bugs have not been modified in over 1 month old. I propose we have a bot that educates people about writing easy to review patches and auto-rejects any patches in bugs that haven't been touched in over a month. I'd love to see a bot that educates people but I'm not sure if we should auto-rejects patches that are simply old. Maybe we need a little bit more of graduality like pinging the author of the patch first, and then make it obsolete if nobody responds etc... This is just an initial proposal. I'm not wed to any of the details of how this would work. I do think that auto-rejecting old patches is valuable to the project as a whole. Having the review queue be so large makes it daunting for any reviewer to try and tackle it. I'd agree that having a really large review queue isn't ideal. Maybe we can customize the review queue so that it only shows patches of your expertise. Best, Ryosuke Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] review queue crazy idea
I've had patches sit in the review queue for 4 weeks then receive a positive review and land without much incident. Some patches are difficult to review or have a limited number of potential reviewers. I would have really appreciated a reminder email about that patch in particular (I honestly had forgotten about it by the time the review email came) but it would not have been helpful to mark it review-. I would have just marked it review? again, which given the eventual outcome (r+ with some comments) would have been the right move. We should make it clearer to new contributors that it is the role of the patch submitter to sell it to the project. This means making it clear why the patch is an improvement, making it easy for the reviewer to understand, and making a convincing argument that the patch is correct by thorough testing. This also tends to mean figuring out who could review a patch and chasing them down via email or IRC. I think if contributors thought in these terms then it would naturally encourage the behaviors reviewers like - breaking patches up, providing more test cases, and clearly explaining what a patch is trying to do. - James On Wed, Jul 21, 2010 at 10:17 PM, Ryosuke Niwa rn...@webkit.org wrote: On Wed, Jul 21, 2010 at 2:40 PM, Ojan Vafai o...@chromium.org wrote: There are currently 38 (of 171 total) patches in the review queue where the bugs have not been modified in over 1 month old. I propose we have a bot that educates people about writing easy to review patches and auto-rejects any patches in bugs that haven't been touched in over a month. I'd love to see a bot that educates people but I'm not sure if we should auto-rejects patches that are simply old. Maybe we need a little bit more of graduality like pinging the author of the patch first, and then make it obsolete if nobody responds etc... This is just an initial proposal. I'm not wed to any of the details of how this would work. I do think that auto-rejecting old patches is valuable to the project as a whole. Having the review queue be so large makes it daunting for any reviewer to try and tackle it. I'd agree that having a really large review queue isn't ideal. Maybe we can customize the review queue so that it only shows patches of your expertise. Best, Ryosuke Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
