If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Black lines appearing on the Opera Desktop Team Blog comments

RESOLVED FIXED

Status

()

Core
Layout
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: Daniel.S, Assigned: Eli Friedman)

Tracking

({regression})

Trunk
x86
All
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.0.11) Gecko/20070312 Firefox/1.5.0.11
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070511 Minefield/3.0a5pre ID:2007051106 [cairo]

On the left side of the comments on the Opera Desktop Team Blog (see the given URL) there are black lines appearing.

The regression window is:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070206 Minefield/3.0a2pre ID:2007020611 [cairo] (No bug visible).

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070207 Minefield/3.0a3pre ID:2007020702 [cairo] (Bug appears: black lines).

The black lines are always to the left of the comments text. Sometimes black lines also appear at the bottom of the last text line.

In earlier builds (2007-02-28-04, which I passed by looking for the regression window) the black line on the left expanded partially to 2px when the text was selected/unselected.

Reproducible: Always

Updated

11 years ago
Component: General → Layout
Keywords: regression
Product: Firefox → Core
QA Contact: general → layout
Version: unspecified → Trunk
Regression Range

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-02-06+11&maxdate=2007-02-07+03&cvsroot=%2Fcvsroot
Blocks: 177805
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

11 years ago
I'm seeing this on Linux, too; maybe OS should be changed to All

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a5pre) Gecko/20070514 Minefield/3.0a5pre ID:2007051404 [cairo]

Updated

11 years ago
OS: Windows XP → All
(Assignee)

Comment 3

11 years ago
Created attachment 265372 [details] [diff] [review]
Patch v1

I think this is the right way to fix this.

I don't like views.
Assignee: nobody → sharparrow1
Status: NEW → ASSIGNED
Attachment #265372 - Flags: review?(roc)
(Assignee)

Updated

11 years ago
Blocks: 370466
The nsIViewObserver change is presumably not part of this patch...

+  nsRect newBounds(NSAppUnitsToIntPixels(viewBounds.x, p2a),
+                   NSAppUnitsToIntPixels(viewBounds.y, p2a),
+                   NSAppUnitsToIntPixels(viewBounds.x + viewBounds.width, p2a),
+                   NSAppUnitsToIntPixels(viewBounds.y + viewBounds.height, p2a));
+
+  newBounds.width -= newBounds.x;
+  newBounds.height -= newBounds.y;

This is slightly evil. In my compositor patch I added a new method to nsRect, ScaleRoundPreservingCenters. Want to adopt that here?

+  NS_ASSERTION(!viewRect.x && !viewRect.y, "When exactly is this supposed to be non-zero?");

When the view has child views that overflow to the left or above. Maybe you want to roll that into mViewToWidgetOffset?
(Assignee)

Comment 5

11 years ago
(In reply to comment #4)
> The nsIViewObserver change is presumably not part of this patch...

That's part of bug 379015... I can check it in anytime, though.

> +  nsRect newBounds(NSAppUnitsToIntPixels(viewBounds.x, p2a),
> +                   NSAppUnitsToIntPixels(viewBounds.y, p2a),
> +                   NSAppUnitsToIntPixels(viewBounds.x + viewBounds.width,
> p2a),
> +                   NSAppUnitsToIntPixels(viewBounds.y + viewBounds.height,
> p2a));
> +
> +  newBounds.width -= newBounds.x;
> +  newBounds.height -= newBounds.y;
> 
> This is slightly evil. In my compositor patch I added a new method to nsRect,
> ScaleRoundPreservingCenters. Want to adopt that here?

I've kinda been wondering about that... are we sure we're not going to get an accuracy loss multiplying by the inverse?  Or do we not care?

> +  NS_ASSERTION(!viewRect.x && !viewRect.y, "When exactly is this supposed to
> be non-zero?");
> 
> When the view has child views that overflow to the left or above. Maybe you
> want to roll that into mViewToWidgetOffset?

Yeah, that's what I thought... I patched in the assertion a few days ago, though, and I haven't managed to hit it, so I haven't figured out how to test it.  (It's probably possible with an XUL deck, though... I wish I could get rid of the evil deck frame hackery, but I haven't quite figured it out.)
> are we sure we're not going to get an accuracy loss multiplying by the inverse?

round(appunits/p2a) should be equal to round(appunits*(1/p2a)), given that appunits is an integer. floor or ceil would be a problem, yes.

> Yeah, that's what I thought... I patched in the assertion a few days ago,
> though, and I haven't managed to hit it, so I haven't figured out how to test
> it.

Now that views are not used for position:fixed, position:absolute and opacity, it's definitely a lot harder. How about overflow:hidden content where there's overflow to the left or above?
(Assignee)

Comment 7

11 years ago
(In reply to comment #6)
> > are we sure we're not going to get an accuracy loss multiplying by the inverse?
> 
> round(appunits/p2a) should be equal to round(appunits*(1/p2a)), given that
> appunits is an integer. floor or ceil would be a problem, yes.

Okay, fine. (I suppose we don't really care about the case where the number of appunits can't be exactly represented as a float... that would be a really long webpage...)

> > Yeah, that's what I thought... I patched in the assertion a few days ago,
> > though, and I haven't managed to hit it, so I haven't figured out how to test
> > it.
> 
> Now that views are not used for position:fixed, position:absolute and opacity,
> it's definitely a lot harder. How about overflow:hidden content where there's
> overflow to the left or above?

Actually, it's more difficult than that: we won't hit the assertion unless the view has a widget, because Refresh only gets called on views with widgets.  The only places that create widgets at the moment are iframes, plugins, scrollable views for overflow: auto divs, tree bodies, popups, and children of decks.  Except for the children of deck frames, all of those clip their overflow.

(I'd really like to get rid of the children of deck frames creating widgets thing, but I've run into issues.)
> (I suppose we don't really care about the case where the number of
> appunits can't be exactly represented as a float

I think we *kinda* care, but not very much. A few rounding-error lines drawn would be OK, crashing or hanging not OK...

Good point about the views with widgets.
(Assignee)

Comment 9

11 years ago
Created attachment 265747 [details] [diff] [review]
Patch v2

New version with nsRect updates from Compositor patch.
Attachment #265372 - Attachment is obsolete: true
Attachment #265747 - Flags: review?(roc)
Attachment #265372 - Flags: review?(roc)
Attachment #265747 - Flags: superreview+
Attachment #265747 - Flags: review?(roc)
Attachment #265747 - Flags: review+
(Assignee)

Comment 10

11 years ago
Checked in.  (Note that none of our current automated tests can catch this.)
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
(Assignee)

Comment 11

11 years ago
It looks like this caused a couple regressions; I'll look into it later.

Updated

11 years ago
Depends on: 381706
(Assignee)

Updated

11 years ago
Depends on: 381746
Depends on: 381957

Updated

11 years ago
Depends on: 382756

Updated

11 years ago
Depends on: 383035
Depends on: 410748
Depends on: 445932
You need to log in before you can comment on or make changes to this bug.