Bug 10116 - REGRESSION: Menu item drawn 2 pixels short on WWDC 2006 Attendee Site
Summary: REGRESSION: Menu item drawn 2 pixels short on WWDC 2006 Attendee Site
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Nobody
URL: https://developer.apple.com/wwdc2006/
Keywords: HasReduction, InRadar, Regression
Depends on: 12123
Blocks:
  Show dependency treegraph
 
Reported: 2006-07-26 01:04 PDT by David Kilzer (:ddkilzer)
Modified: 2007-02-18 12:44 PST (History)
4 users (show)

See Also:


Attachments
Screenshot showing menu issue (29.84 KB, image/png)
2006-07-26 01:07 PDT, David Kilzer (:ddkilzer)
no flags Details
Partial reduction test case (2.41 KB, text/html)
2006-07-26 01:32 PDT, David Kilzer (:ddkilzer)
no flags Details
Further reduction (510 bytes, text/html)
2006-07-26 12:03 PDT, mitz
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2006-07-26 01:04:16 PDT
Steps to reproduce:

1. Open WebKit nightly.
2. Log into WWDC 2006 Attendee Site.  https://developer.apple.com/wwdc2006/

Expected results:

The green menu should be the same height all the way across.

Actual results:

The green menu is 2 pixels short on the "My Agenda" menu item.

Regression:

The menu draws correctly on production Safari 2.0.4 (419.3) on Mac OS X 10.4.7 (8J135/PowerPC).  Firefox 1.5.0.4 also draws the menu correctly on the same OS X.

Notes:

Tested with locally-built debug build of WebKit r15633.
Comment 1 David Kilzer (:ddkilzer) 2006-07-26 01:07:41 PDT
Created attachment 9693 [details]
Screenshot showing menu issue
Comment 2 David Kilzer (:ddkilzer) 2006-07-26 01:32:05 PDT
Created attachment 9694 [details]
Partial reduction test case
Comment 3 mitz 2006-07-26 11:44:42 PDT
I think this is a regression from the shadow repaint bug fix(es), bug 7301. Shadow is added to the inlines' overflow rect, and table cells are sized to contain all overflows. I don't know how to fix it :-(
Comment 4 mitz 2006-07-26 12:03:34 PDT
Created attachment 9701 [details]
Further reduction

The padding-bottom: 1px on the span is necessary. I don't know why.
Comment 5 Dave Hyatt 2006-07-26 13:04:47 PDT
The problem is there's visual overflow and layout overflow.  (The former includes the latter but not vice versa).  Although it's annoyingly complicated, maybe we need to separate the two (and have two rects instead of just one).
Comment 6 Dave Hyatt 2006-07-26 13:07:27 PDT
The custom highlight code I just added in my last check-in has the same issue.  It's intended to be purely visual overflow and should not have affected layout.  So shadow and highlight are both suffering from this problem now.

Comment 7 Stephanie Lewis 2007-01-27 19:41:38 PST
Radar <rdar://problem/4959690>
Comment 8 mitz 2007-02-09 12:33:03 PST
The patch I just posted for bug 12123 (attachment 13090 [details]) fixes this bug as well.
Comment 9 John Sullivan 2007-02-09 16:15:42 PST
Even more reduction:

<table style="border:1px solid black;">
<tr><td style="border:1px solid blue;text-shadow:gray 100px 100px;">Shadowed Text</td></tr>
</table>		

This shows that on tip of tree, the vertical (but not horizontal) shadow offset is affecting the height of the enclosing td, and thus the enclosing table. In released Safari, the td and table size ignore the shadow offset.

This doesn't happen on browsers that don't support text-shadow, of course.

I don't know what the expected behavior is here, but it seems clear that what we're doing now is wrong. One of the following seems like it must be true:

1) the shadow offset should not affect the enclosing td size at all
2) the shadow offset should affect both the width and height of the enclosing td

If (1) is true, then this is a regression from correct behavior in Safari 2.0.4. If (2) is true, then the change is part-way towards the correct behavior.
Comment 10 mitz 2007-02-12 16:24:11 PST
The patch for bug 12123 (r19588) fixed this bug as well.
Comment 11 mitz 2007-02-17 22:51:59 PST
Reopening because r19588 was rolled out.