Bug 97087

Summary:

[EFL][DRT]DumpRenderTree needs to reset focus state when test starts.

Product:

WebKit

Reporter:

Michal Pakula vel Rutka <mpakulavelrutka>

Component:

WebKit EFL

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

g.czajkowski, gyuyoung.kim, lucas.de.marchi, rakuco, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

Linux

Attachments:

Description	Flags
proposed patch	none
setting focus moved into resetDefaultsToConsistentValues	none
change log fixes	none
moved focusing function below	none

Description Michal Pakula vel Rutka 2012-09-19 02:17:43 PDT

When running layout tests in editing/undo category in WebKit1 editing/undo/undo-indent and editing/undo/undo-misspellings have flaky results, as editing delegates does not match expected one. However if those test where ran separately, results where as expected. The difference comes from running editing/undo/undo-iframe-location-change before tests mentioned above: this test leaves frame in unfocused state and proper delegates are not triggered.

Comment 1 Michal Pakula vel Rutka 2012-09-19 02:50:27 PDT

Created attachment 164701 [details]
proposed patch

The patch adds main frame focusing similar to WebKit-GTK's DumpRenderTree view focusing in dump() function. Then tests will start with main frame focused regardless of earlier test's frame focus state.

Comment 2 Grzegorz Czajkowski 2012-09-19 03:14:08 PDT

LGTM.

Just wondering, did you try to set the frame focus in DumpRenderTreeChrome::resetDefaultsToConsistentValues()? IMHO it's good place to restore the original settings before running test.

Comment 3 Michal Pakula vel Rutka 2012-09-19 04:33:55 PDT

(In reply to comment #2)
> LGTM.
> 
> Just wondering, did you try to set the frame focus in DumpRenderTreeChrome::resetDefaultsToConsistentValues()? IMHO it's good place to restore the original settings before running test.

I checked and it also fixes the problem. I will check if it does not introduce any regressions, and then post a new patch.

Comment 4 Michal Pakula vel Rutka 2012-09-19 09:08:30 PDT

Created attachment 164746 [details]
setting focus moved into resetDefaultsToConsistentValues

Comment 5 Grzegorz Czajkowski 2012-09-19 23:41:08 PDT

Comment on attachment 164746 [details]
setting focus moved into resetDefaultsToConsistentValues

View in context: https://bugs.webkit.org/attachment.cgi?id=164746&action=review

Thanks for your verification of newly proposed approach and changes. Looks better.

> LayoutTests/ChangeLog:8
> +        Remove two flaky tests from TestExpectations.

Please add a note that after setting frame focus the tests are passing now.

> Tools/ChangeLog:8
> +        Add focusing a main frame on each test dump.

on each test dump -> on settings reset?

Comment 6 Michal Pakula vel Rutka 2012-09-20 01:28:10 PDT

Created attachment 164859 [details]
change log fixes

Comment 7 Gyuyoung Kim 2012-09-20 01:39:33 PDT

Comment on attachment 164859 [details]
change log fixes

View in context: https://bugs.webkit.org/attachment.cgi?id=164859&action=review

> Tools/DumpRenderTree/efl/DumpRenderTreeChrome.cpp:291
>      ewk_view_text_direction_set(mainView(), EWK_TEXT_DIRECTION_DEFAULT);

It would be good if you add a new line to here.

Comment 8 Michal Pakula vel Rutka 2012-09-20 02:09:14 PDT

Created attachment 164864 [details]
moved focusing function below

Comment 9 Gyuyoung Kim 2012-09-20 02:11:19 PDT

Comment on attachment 164864 [details]
moved focusing function below

Looks fine.

Comment 10 Grzegorz Czajkowski 2012-09-20 02:53:51 PDT

Comment on attachment 164864 [details]
moved focusing function below

LGTM. Thanks.

Comment 11 WebKit Review Bot 2012-09-20 03:16:14 PDT

Comment on attachment 164864 [details]
moved focusing function below

Clearing flags on attachment: 164864

Committed r129110: <http://trac.webkit.org/changeset/129110>

Comment 12 WebKit Review Bot 2012-09-20 03:16:18 PDT

All reviewed patches have been landed.  Closing bug.