Summary: | REGRESSION (r79762): Items in <select multiple> have focus rings, but shouldn't | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Roben (:aroben) <aroben> | ||||||||
Component: | Forms | Assignee: | Chang Shu <cshu> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | adele, cshu, mrobinson, ossy, rniwa, tonikitoo | ||||||||
Priority: | P2 | Keywords: | InRadar, Regression | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Windows XP | ||||||||||
Attachments: |
|
Description
Adam Roben (:aroben)
2011-02-27 10:11:39 PST
Sorry, I will work on it. Created attachment 84084 [details]
fix patch
Comment on attachment 84084 [details] fix patch View in context: https://bugs.webkit.org/attachment.cgi?id=84084&action=review > LayoutTests/fast/forms/select-listbox-multiple-no-focusring.html:17 > + document.write("To manually test, click on the 'a' option and no focus ring on the selected item should be seen."); I would also had added a test with spatial navigation on, so we would garantee both behaviors. Comment on attachment 84084 [details] fix patch Clearing flags on attachment: 84084 Committed r79900: <http://trac.webkit.org/changeset/79900> Created attachment 84106 [details]
addtional patch that adds a new test case
this patch draws a focus ring with snav enabled.
Ossy, do you mind generate expected results on Qt as a followup after my patches are landed? (In reply to comment #7) > Ossy, do you mind generate expected results on Qt as a followup after my patches are landed? And Martin, could you help with gtk results? (In reply to comment #7) > Ossy, do you mind generate expected results on Qt as a followup after my patches are landed? Sure. Sure, I'll make sure the GTK+ results are added. Comment on attachment 84106 [details] addtional patch that adds a new test case Clearing flags on attachment: 84106 Committed r79934: <http://trac.webkit.org/changeset/79934> The test added in this patch has been crashing on GTK. Could you investigate it? (In reply to comment #12) > The test added in this patch has been crashing on GTK. Could you investigate it? What?! Yes, I will take a look right away. I wasn't able to build gtk on my ubuntu. I tried to follow the instruction on this page: https://trac.webkit.org/wiki/BuildingGtk. But the first step failed: [cshu@webkit.gtk] sudo apt-add-repository ppa:martin-james-robinson/webkitgtk Error reading https://launchpad.net/api/1.0/~martin-james-robinson/+archive/webkitgtk: <urlopen error [Errno 110] Connection timed out> (In reply to comment #14) > I wasn't able to build gtk on my ubuntu. I tried to follow the instruction on this page: https://trac.webkit.org/wiki/BuildingGtk. But the first step failed: > [cshu@webkit.gtk] sudo apt-add-repository ppa:martin-james-robinson/webkitgtk > Error reading https://launchpad.net/api/1.0/~martin-james-robinson/+archive/webkitgtk: <urlopen error [Errno 110] Connection timed out> That's odd. Are you using Maverick? Are you sure the tests crashing on GTK+ are ones added in this patch? We have a test crashing with a very similar name. It look like those failures are tracked by this bug: https://bugs.webkit.org/show_bug.cgi?id=53146 > > [cshu@webkit.gtk] sudo apt-add-repository ppa:martin-james-robinson/webkitgtk > > Error reading https://launchpad.net/api/1.0/~martin-james-robinson/+archive/webkitgtk: <urlopen error [Errno 110] Connection timed out> > > That's odd. Are you using Maverick? It was the proxy setting issues. I manually added the repository and it worked. > Are you sure the tests crashing on GTK+ are ones added in this patch? We have a test crashing with a very similar name. It look like those failures are tracked by this bug: https://bugs.webkit.org/show_bug.cgi?id=53146 I just finished building and ran the two tests I added in the two patches in this bug. They all passed successfully. Niwa, did you see the crash on buildbot? Created attachment 84246 [details]
fix patch: gtk expected results
I attached the gtk results as it comes handy.
Comment on attachment 84246 [details] fix patch: gtk expected results I'm afraid these pixel results aren't quite right. You'll need to run run-webkit-test in Xvfb and with "env -i". See: https://trac.webkit.org/wiki/WebKitGtkLayoutTests Thanks for looking into this! (In reply to comment #18) > (From update of attachment 84246 [details]) > I'm afraid these pixel results aren't quite right. You'll need to run run-webkit-test in Xvfb and with "env -i". See: https://trac.webkit.org/wiki/WebKitGtkLayoutTests Thanks for looking into this! I'll add them now. Sorry I didn't take care of it yesterday. (In reply to comment #19) > (In reply to comment #18) > > (From update of attachment 84246 [details] [details]) > > I'm afraid these pixel results aren't quite right. You'll need to run run-webkit-test in Xvfb and with "env -i". See: https://trac.webkit.org/wiki/WebKitGtkLayoutTests Thanks for looking into this! > > I'll add them now. Sorry I didn't take care of it yesterday. Sure. thanks for the help! I don't think we have to keep this open anymore. The platform specific results can be landed without review. Landed results for GTK+ here: http://trac.webkit.org/changeset/80016 |