Remove mHasCachedOutline

RESOLVED FIXED in Firefox 48

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

(Blocks: 1 bug)

unspecified
mozilla48
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
outline-width never takes a percentage, so this is always true.
(Assignee)

Comment 1

2 years ago
Created attachment 8743602 [details] [diff] [review]
Remove mHasCachedOutline. v1
Comment on attachment 8743602 [details] [diff] [review]
Remove mHasCachedOutline. v1

> nsComputedDOMStyle::DoGetOutlineWidth()
> {
>   RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
> 
>   const nsStyleOutline* outline = StyleOutline();
> 
>   nscoord width;
>   if (outline->GetOutlineStyle() == NS_STYLE_BORDER_STYLE_NONE) {
>-    NS_ASSERTION(outline->GetOutlineWidth(width) && width == 0,
>-                 "unexpected width");

You should rewrite this assertion (outline->GetOutlineWidth() == 0) rather than deleting.

> void 
> nsStyleOutline::RecalcData(nsPresContext* aContext)
> {
>   if (NS_STYLE_BORDER_STYLE_NONE == GetOutlineStyle()) {
>     mCachedOutlineWidth = 0;
>-    mHasCachedOutline = true;
>   } else if (IsFixedUnit(mOutlineWidth, true)) {

This IsFixedUnit() check should become an assertion that IsFixedUnit() is true, rather than a test.

>@@ -1286,23 +1286,20 @@ struct nsStyleOutline
...
>-  bool GetOutlineWidth(nscoord& aWidth) const
>+  nscoord GetOutlineWidth() const
>   {
>-    if (mHasCachedOutline) {
>-      aWidth = mCachedOutlineWidth;
>-      return true;
>-    }
>-    return false;
>+    MOZ_ASSERT(GetOutlineStyle() != NS_STYLE_BORDER_STYLE_NONE);
>+    return mCachedOutlineWidth;
>   }

Please don't add this MOZ_ASSERT; this should be usable (and return 0) when the outline-style is none.


r=dbaron with that
Attachment #8743602 - Flags: review+

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/413ad396d8a4
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.