Bug 72002 – Add NULL checks to setting access which is obtained on frame and document

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Grzegorz Czajkowski]

Adds NULL checks to seeting object where it's required.
Generally WebCore checks NULL which may be returned from setting object obtained on frame or document but in some cases these are skipped. These checks are not needed to setting's access on page object.

[Grzegorz Czajkowski]

Created attachment 114444 [details]
proposed patch

[Early Warning System Bot]

Comment on attachment 114444 [details]
proposed patch

Attachment 114444 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10354296

[WebKit Review Bot]

Comment on attachment 114444 [details]
proposed patch

Attachment 114444 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10401241

New failing tests:
editing/input/password-echo-passnode.html
editing/input/password-echo-passnode2.html

[Grzegorz Czajkowski]

Created attachment 115133 [details]
updated patch

Fix WebKit-Qt build break

[WebKit Review Bot]

Comment on attachment 115133 [details]
updated patch

Attachment 115133 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10483285

New failing tests:
editing/input/password-echo-passnode.html
editing/input/password-echo-passnode2.html

[Grzegorz Czajkowski]

Hello Dimitri and Nate,

Can you confirm that this patch may cause failing of those tests? I don't have environment with Chromium DumpRenderTree application so I'd glad to hear your opinion.
Thanks.

[Eric Seidel (no email)]

Comment on attachment 115133 [details]
updated patch

OK.  Can we test this?

[Ryosuke Niwa]

(In reply to comment #5)
> (From update of attachment 115133 [details])
> Attachment 115133 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/10483285
> 
> New failing tests:
> editing/input/password-echo-passnode.html
> editing/input/password-echo-passnode2.html

Your patch probably breaks these two tests on all platforms (note: only cr-linux EWS bots run tests).

[Grzegorz Czajkowski]

(In reply to comment #8)
> (In reply to comment #5)
> > (From update of attachment 115133 [details] [details])
> > Attachment 115133 [details] [details] did not pass chromium-ews (chromium-xvfb):
> > Output: http://queues.webkit.org/results/10483285
> > 
> > New failing tests:
> > editing/input/password-echo-passnode.html
> > editing/input/password-echo-passnode2.html
> 
> Your patch probably breaks these two tests on all platforms (note: only cr-linux EWS bots run tests).

CC'ing Andreas Kling.
Ok, Andreas has found bug in the patch.
I will fix it.

[Grzegorz Czajkowski]

Created attachment 121815 [details]
updated patch

Fixes bug which caused failing of tests.

[Andreas Kling]

Comment on attachment 121815 [details]
updated patch

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

> Source/WebCore/ChangeLog:8
> +        Adds NULL checks to seeting object where it's required.

Typo, settings.

[Grzegorz Czajkowski]

Created attachment 121823 [details]
patch for landing

Fixed typo.

[Andreas Kling]

For the record, it's unfortunate that we don't have tests for these changes. However I believe these null-checks are sane and belong in WebKit.

In the future, please provide tests that exercise your changes, or explain why tests aren't possible.

[WebKit Review Bot]

Comment on attachment 121823 [details]
patch for landing

Clearing flags on attachment: 121823

Committed r104552: <http://trac.webkit.org/changeset/104552>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.