Bug 37141 - [Chromium] Crasher in popup menu code
Summary: [Chromium] Crasher in popup menu code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-06 00:33 PDT by Jay Civelli
Modified: 2010-04-07 14:11 PDT (History)
2 users (show)

See Also:


Attachments
Fix potential NULL pointer read. (1.09 KB, patch)
2010-04-06 00:40 PDT, Jay Civelli
no flags Details | Formatted Diff | Diff
Adding a layout test (3.56 KB, patch)
2010-04-06 18:27 PDT, Jay Campan
jianli: review-
Details | Formatted Diff | Diff
Applying suggested changes. (3.56 KB, patch)
2010-04-07 12:02 PDT, Jay Campan
jianli: review-
Details | Formatted Diff | Diff
Added Chromium mention in ChangeLog files. (deleted)
2010-04-07 13:20 PDT, Jay Campan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jay Civelli 2010-04-06 00:33:03 PDT
1. Go to search.twitter.com
2. Enter "toto" into the search box, hit enter

An illegal read happens in PopupMenu::~PopupMenu().
Comment 1 Jay Civelli 2010-04-06 00:40:24 PDT
Created attachment 52614 [details]
Fix potential NULL pointer read.
Comment 2 Jian Li 2010-04-06 13:24:36 PDT
Is it possible to add a layout test for this?
Comment 3 Jay Campan 2010-04-06 18:27:19 PDT
Created attachment 52692 [details]
Adding a layout test
Comment 4 Jian Li 2010-04-07 11:10:15 PDT
Comment on attachment 52692 [details]
Adding a layout test

The bug title should be prefixed with "[Chromium]" to indicate that this is for chromium only.

> --- LayoutTests/ChangeLog	(revision 57181)
> +++ LayoutTests/ChangeLog	(working copy)
> @@ -1,3 +1,14 @@
> +2010-04-06  Jay Civelli  <jcivelli@chromium.org>
Garbage characters at the beginning.

> --- LayoutTests/platform/chromium/fast/forms/search-popup-crasher.html	(revision 0)
> +++ LayoutTests/platform/chromium/fast/forms/search-popup-crasher.html	(revision 0)
> @@ -0,0 +1,27 @@
> +<HTML>
> +<HEAD>
> +<script>
> +window.onload = function()
> +{
> +    if (window.layoutTestController)
> +        layoutTestController.dumpAsText();
> +
> +
Please remove the extra empty line.

> +    if (window.location.href.indexOf('formSubmitted') != -1)
> +        return;
> +    document.getElementById("searchBox").value= 'quentin';
Might be better to stick with double-quote than single-quote, for consistency.

> +    document.getElementById("searchForm").submit();
> +};
> +</script>
> +</HEAD>
> +<BODY>
> +
> +<p>This page tests that a page with a search popup does not crash, as reported in <a href='https://bugs.webkit.org/show_bug.cgi?id=37141'>this bug</a>. Below is a search input that the test automatically fills than submit. Pass if this does not crash.</p><br><br>
Typo: than => then
Comment 5 Jay Campan 2010-04-07 12:02:42 PDT
Created attachment 52766 [details]
Applying suggested changes.
Comment 6 Jian Li 2010-04-07 13:09:17 PDT
Comment on attachment 52766 [details]
Applying suggested changes.

Please update both ChangeLog to reflect the bug title change.

Do you want the commit-queue to land it for you? If so, please specify "?" for the "commit-queue" flag when you submit your new patch.
Comment 7 Jay Campan 2010-04-07 13:20:41 PDT
Created attachment 52777 [details]
Added Chromium mention in ChangeLog files.
Comment 8 Jian Li 2010-04-07 13:53:57 PDT
Comment on attachment 52777 [details]
Added Chromium mention in ChangeLog files.

r=me
Comment 9 WebKit Commit Bot 2010-04-07 14:11:06 PDT
Comment on attachment 52777 [details]
Added Chromium mention in ChangeLog files.

Clearing flags on attachment: 52777

Committed r57230: <http://trac.webkit.org/changeset/57230>
Comment 10 WebKit Commit Bot 2010-04-07 14:11:11 PDT
All reviewed patches have been landed.  Closing bug.