Bug 38383
| Summary: | [Chromium] r58564 exposed that some fast/forms/search-* tests try click incorrect locations to click cancel buttons | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Fumitoshi Ukai <ukai> |
| Component: | Forms | Assignee: | Kent Tamura <tkent> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | abarth, bulach, dglazkov, eric, jorlow, tkent, webkit.review.bot, yaar |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | PC | ||
| OS: | OS X 10.5 | ||
Fumitoshi Ukai
Since WebKit blamelist r58563:r58565 or Chrome blamelist r46050:r46050, two layout tests started to fail.
fast/forms/search-abs-pos-cancel-button.html
Can't click the clear button of an absolutely positioned search field.
and FAILed.
fast/forms/search-cancel-button-mouseup.html
FAIL.
fast/forms/search-rtl.html
FAIL.
fast/forms/search-zoomed.html
FAIL.
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#useWebKitCanary=true&tests=fast%2Fforms%2Fsearch-abs-pos-cancel-button.html%2Cfast%2Fforms%2Fsearch-cancel-button-mouseup.html%2Cfast%2Fforms%2Fsearch-rtl.html%2Cfast%2Fforms%2Fsearch-zoomed.html
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Jeremy Orlow
This is definitely not enough information. Why did you put these in test expectations? Do we expect these to fail for some good reason? Otherwise you're making it easier for us to roll in a major regression on the next roll.
Kent Tamura
Ukai-san and I talked locally, and I'm fixing this regression.
Jeremy Orlow
That would have been great information to include in the bug.
I still don't understand why these test expectations were added tho. We don't plan on rolling these failures into Chrome on the next roll, right?
Kent Tamura
(In reply to comment #3)
> That would have been great information to include in the bug.
>
> I still don't understand why these test expectations were added tho. We don't
> plan on rolling these failures into Chrome on the next roll, right?
(Ukai-san left the office. I'm not sure Ukai-san's intention correctly)
I think these failures are ok to be included in the next roll.
Basically these tests are wrong, and r58564's behavior in Chrome will have no problem.
Jeremy Orlow
As long as it's the test, not the implementation that's wrong then we're good.
Would you mind updating this bug with an explanation of what the problem is, what you're planning on doing to fix it, etc. Also update the bug title? I know you're taking care of it, but if nothing else, it's important to make sure that someone can come back to this 3 months later and clearly know what happened and what was done to fix it. It won't take more than 2 minutes.
Kent Tamura
Committed a fix for this as r58572.
* rendering/RenderTextControlSingleLine.cpp:
(WebCore::RenderTextControlSingleLine::forwardEvent):
r58564 made a region check for the cancel button stricter, but it
made some tests failing on Chromium. So, relax the check again.
WebKit Review Bot
http://trac.webkit.org/changeset/58572 might have broken GTK Linux 32-bit Debug