Bug 7691 - REGRESSION: imdb.com search button looks wrong because "Submit" is drawn
Summary: REGRESSION: imdb.com search button looks wrong because "Submit" is drawn
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P1 Normal
Assignee: Tim Omernick
URL: http://imdb.com/
Keywords: EasyFix, HasReduction, InRadar, Regression
Depends on:
Blocks:
 
Reported: 2006-03-09 19:52 PST by Daniele Metilli
Modified: 2006-05-04 08:32 PDT (History)
1 user (show)

See Also:


Attachments
Test case (292 bytes, text/html)
2006-03-09 19:57 PST, Daniele Metilli
no flags Details
Better reduction (407 bytes, text/html)
2006-03-10 00:10 PST, Daniele Metilli
no flags Details
Patch w/ fix and layout test (7.41 KB, patch)
2006-03-23 15:54 PST, Tim Omernick
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniele Metilli 2006-03-09 19:52:51 PST
The yellow button in the "Search the IMDb" section on the left does not render correctly. 

This is the problem:
- The background image already has a text.
- WebKit tries to add another text on top of it (the "value" attribute or, if this is empty, "Submit").

Maybe a check should be made to ensure that the Aqua button is not used when the CSS class of the button has a "background" value?
Comment 1 Daniele Metilli 2006-03-09 19:57:08 PST
Created attachment 6972 [details]
Test case

I have added a test case that shows two example buttons.
Comment 2 Daniele Metilli 2006-03-10 00:10:21 PST
Created attachment 6979 [details]
Better reduction
Comment 3 Gustaaf Groenendaal (MysteryQuest) 2006-03-10 01:02:21 PST
Confirmed with release 13202 and 13244, setting severity to normal in stead of minor, since some pages on some sites could be unaccesable. Also set priority to P1 since this is a regression bug and added to the summary.
Comment 4 Darin Adler 2006-03-12 22:00:31 PST
The difference from Firefox seems to be that we display "Submit" rather than an empty string. Perhaps the fix is to supply "Submit" only when there is no value specified. If so, this can be fixed by simply changing HTMLInputElementImpl::valueWithDefault to supply the default only if isNull rather than isEmpty. The key would be finding test cases.
Comment 5 Alice Liu 2006-03-20 07:39:14 PST
<rdar://problem/4483867>
Comment 6 Tim Omernick 2006-03-23 15:21:20 PST
I'm taking this one.  I've got a fix and a layout test.  Will submit for review soon.
Comment 7 Tim Omernick 2006-03-23 15:54:57 PST
Created attachment 7264 [details]
Patch w/ fix and layout test
Comment 8 Darin Adler 2006-03-23 16:48:01 PST
Comment on attachment 7264 [details]
Patch w/ fix and layout test

r=me
Comment 9 Tim Omernick 2006-03-23 17:48:37 PST
Fix landed to TOT, r13464.
Comment 10 Daniele Metilli 2006-05-01 06:38:54 PDT
I'm reopening this bug since it still happens in this case:

1 - Go to http://imdb.com/title/tt0120907/
2 - Click on "David Cronenberg".
3 - Go back using Safari's back button.
4 - The search button is rendered incorrectly.

This happens in TOT (r14126).
Comment 11 Darin Adler 2006-05-01 08:07:55 PDT
Please don't reopen a bug when the original site now works fine and there's a test case that also still works.

A new bug report is much better for cases like that!
Comment 12 Darin Adler 2006-05-01 08:09:41 PDT
The new bug doesn't have to do with the button rendering itself, but rather with the way form data is saved and restored. Apparently a null is saved, but an empty string is restored, or vice versa.
Comment 13 Darin Adler 2006-05-01 08:10:13 PDT
Another reason to not reuse the original bug report is that this now shows up in the list of bugs with fixes to be landed.
Comment 14 Darin Adler 2006-05-01 08:23:27 PDT
It would be best to change this bug back to FIXED and open a new bug about the fact that this happens after going back.

The fix will be to change HTMLInputElement::state and HTMLInputElement::restoreState so they can distinguish a state of null string from a state of empty string, which should be straightforward.
Comment 15 Daniele Metilli 2006-05-01 09:19:24 PDT
Sorry about that, I thought that since it was so strictly related (same site, same button) it could be that the fix was incomplete. I didn't imagine that it was a completely different problem. I will now do exactly as you said and report a new bug. Thanks for your help :)
Comment 16 Darin Adler 2006-05-04 08:32:48 PDT
Follow-up bug is in bug 8683.