The default bug view has changed. See this FAQ.

Using box-shadow on a <fieldset> with <legend> does not follow box shape

RESOLVED FIXED in mozilla23

Status

()

Core
Layout: Block and Inline
RESOLVED FIXED
8 years ago
3 years ago

People

(Reporter: Martijn Otto, Assigned: mats)

Tracking

({testcase})

Trunk
mozilla23
testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(7 attachments, 6 obsolete attachments)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b3) Gecko/20090307 Ubuntu/8.10 (intrepid) Shiretoko/3.1b3
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b3) Gecko/20090307 Ubuntu/8.10 (intrepid) Shiretoko/3.1b3

Using -moz-box-shadow on a fieldset will show shadow as if the legend continues to the right of the fieldset.

Reproducible: Always

Steps to Reproduce:
1. Build a form with a fieldset
2. Add -moz-box-shadow style rule
Actual Results:  
Shadow flows too wide around the fieldset

Expected Results:  
Shadow flows nicely around the fieldset
(Reporter)

Comment 1

8 years ago
Created attachment 369245 [details]
testcase
Version: unspecified → 3.1 Branch

Comment 2

8 years ago
This bug is also present in the Windows version of Firefox.
(Reporter)

Updated

8 years ago
OS: Linux → All
Hardware: x86_64 → All

Comment 3

8 years ago
Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090920 Minefield/3.7a1pre
Status: UNCONFIRMED → NEW
Component: General → Layout: View Rendering
Ever confirmed: true
Product: Firefox → Core
QA Contact: general → layout.view-rendering
Version: 3.5 Branch → Trunk

Comment 4

8 years ago
Changing the summary to describe the problem better.

When fieldset is used with legend the border updates its position. Maybe if the shadow follows the border this problem will gone.
Blocks: 212633
Summary: Using -moz-box-shadow on a fieldset adds shadow for legend → Using -moz-box-shadow on a fieldset with legend does not follow box shape

Comment 5

8 years ago
Created attachment 401830 [details]
testcase 2

Testcase with backgrounds for detailed view.
The body background is above the shadow, which looks very strange.
Attachment #369245 - Attachment is obsolete: true

Comment 6

8 years ago
Created attachment 402192 [details]
simple shadow glitch

top-right shadow is wrong

Updated

8 years ago
Keywords: testcase

Updated

7 years ago
Duplicate of this bug: 568836

Updated

7 years ago
Duplicate of this bug: 593502
Is somebody interested in fixing this one ?

Comment 10

6 years ago
Not capable of fixing this, but it presumably relates to the way the legend gets rendered. It can be fixed in css by positioning the legend absolutely (relative to the fieldset), so that the legend doesn't stretch the background up the top.

fieldset { position: relative; }
fieldset legend { position: absolute; top: -10px; left: 0; }

top & left values depend on fieldset padding.

The shadow needs to be rendered with the border's frame not the fieldset frame, which gets stretched by the legend. Hope this helps???

Comment 11

6 years ago
This bug is still present in 8.0
Note: when we drop -moz-box-shadow in favor of box-shadow (which we've supported for a while), the testcases will need to be modified to omit the prefix.

Updated

5 years ago
Duplicate of this bug: 705130

Comment 14

5 years ago
Created attachment 576777 [details]
testcase

Updated

5 years ago
Attachment #401830 - Attachment is obsolete: true

Updated

5 years ago
Attachment #402192 - Attachment is obsolete: true

Comment 15

5 years ago
(In reply to David Baron [:dbaron] from comment #12)
> testcases will need to be modified to omit the prefix.
done
Summary: Using -moz-box-shadow on a fieldset with legend does not follow box shape → Using box-shadow on a <fieldset> with <legend> does not follow box shape

Comment 16

5 years ago
Well, -moz-box-shadow has apparently been dropped as of version 14 but this bug still hasn't been fixed.  I'm not going to use position:absolute for my legends as that will affect layout.  I shouldn't have to, because Webkit has handled this correctly for years now.  Without -moz-box-shadow, there is no straightforward way to fix this anymore.  Please either bring back -moz-box-shadow or fix this bug.
(Assignee)

Updated

4 years ago
Duplicate of this bug: 854159
(Assignee)

Updated

4 years ago
Component: Layout: View Rendering → Layout: Block and Inline
Probably the way to fix this is to change our implementation of fieldset so that the border box corresponds to where the border is, and the legend's overhang is in the margin area (which we ensure exists).  (Hopefully the legend is already correctly added to the overflow areas, which ensures that scrolling and repainting work correctly, though there's a chance it's not.)
Created attachment 728769 [details]
testcase + layout emulation

What can be the concept of shadow around the legend element?

Updated

4 years ago
Attachment #728769 - Attachment mime type: text/plain → text/html
Created attachment 728797 [details]
browser comparison screenshot (Win)
(Assignee)

Comment 21

4 years ago
Created attachment 730479 [details]
overflow:hidden test

(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #18)
> Probably the way to fix this is to change our implementation of fieldset so
> that the border box corresponds to where the border is, and the legend's
> overhang is in the margin area (which we ensure exists).

Seeing how other UAs clip overflow:hidden, the border boxes looks
the same in Gecko, Trident, Presto, WebKit.  It looks like they just
paint the shadow with the same negative offset as the border.
Wouldn't that be quite simple to implement for us too?
(Assignee)

Comment 22

4 years ago
Created attachment 730503 [details] [diff] [review]
WIP

This gives the desired rendering for the shadow.
(Assignee)

Comment 23

4 years ago
Created attachment 730505 [details] [diff] [review]
WIP

Forgot to include the header file.
Attachment #730503 - Attachment is obsolete: true
(Assignee)

Comment 24

4 years ago
Created attachment 731563 [details] [diff] [review]
fix

This renders the box-shadow in the same way as other UAs.
https://tbpl.mozilla.org/?tree=Try&rev=a1eb793ef0a0

(let's sort out the clipping / border box issues in bug 770645)
Assignee: nobody → matspal
Attachment #730505 - Attachment is obsolete: true
Attachment #731563 - Flags: review?(roc)
Comment on attachment 731563 [details] [diff] [review]
fix

Review of attachment 731563 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/forms/nsFieldSetFrame.cpp
@@ +575,5 @@
> +  if (topBorder < mLegendRect.height) {
> +    yoff = (mLegendRect.height - topBorder)/2;
> +  }
> +  return yoff;
> +}

How about instead creating a new nsIFrame method GetVisualBorderRect which returns mRect normally but is modified for nsFieldSetFrame? Then we don't need to check for nsFieldSetFrame specifically.
Attachment #731563 - Flags: review?(roc) → review+
Comment on attachment 731563 [details] [diff] [review]
fix

Review of attachment 731563 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, didn't mean to r+ yet.

Also, I think nsLayoutUtils::GetBoxShadowRectForFrame doesn't need to include this treatment.
Attachment #731563 - Flags: review+ → review?
(Assignee)

Comment 27

4 years ago
Created attachment 734293 [details] [diff] [review]
fix+test

https://tbpl.mozilla.org/?tree=Try&rev=8da8879637d8
(the JP orange is unrelated), the Windows orange is a few anti-aliased
pixels at the curved border edges.  I added some fuzz to allow that:
https://tbpl.mozilla.org/?tree=Try&rev=f2315c64a863
Attachment #731563 - Attachment is obsolete: true
Attachment #731563 - Flags: review?
Attachment #734293 - Flags: review?(roc)
Attachment #734293 - Flags: review?(roc) → review+
(Assignee)

Comment 28

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f58ed47cee2
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/9f58ed47cee2
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
It might be good to have a test for inset shadows as well.
Flags: needinfo?(matspal)
Created attachment 740534 [details]
inset shadow testcase

Updated

4 years ago
Attachment #740534 - Attachment mime type: text/plain → text/html
(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #30)
> It might be good to have a test for inset shadows as well.

passed, and passed by cross UA compatibility as well at me.
Created attachment 740544 [details]
inset UA comparison screenshot
I meant a reftest committed to the tree and running in our automated test suite.
(Assignee)

Comment 35

3 years ago
Added a reftest with inset box-shadow.
https://hg.mozilla.org/integration/mozilla-inbound/rev/8cee060901b8
Flags: needinfo?(mats)
https://hg.mozilla.org/mozilla-central/rev/8cee060901b8
You need to log in before you can comment on or make changes to this bug.