Bug 97087 - [EFL][DRT]DumpRenderTree needs to reset focus state when test starts.
Summary: [EFL][DRT]DumpRenderTree needs to reset focus state when test starts.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P3 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-19 02:17 PDT by Michal Pakula vel Rutka
Modified: 2012-09-20 03:16 PDT (History)
5 users (show)

See Also:


Attachments
proposed patch (2.53 KB, patch)
2012-09-19 02:50 PDT, Michal Pakula vel Rutka
no flags Details | Formatted Diff | Diff
setting focus moved into resetDefaultsToConsistentValues (2.76 KB, patch)
2012-09-19 09:08 PDT, Michal Pakula vel Rutka
no flags Details | Formatted Diff | Diff
change log fixes (2.85 KB, patch)
2012-09-20 01:28 PDT, Michal Pakula vel Rutka
no flags Details | Formatted Diff | Diff
moved focusing function below (2.74 KB, patch)
2012-09-20 02:09 PDT, Michal Pakula vel Rutka
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.