Closed Bug 46446 Opened 25 years ago Closed 24 years ago

canvas background uses canvas 'origin' not HTML origin [BG]

Categories

(Core :: CSS Parsing and Computation, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla0.9.4

People

(Reporter: ian, Assigned: ian)

References

()

Details

(Keywords: css1, regression, testcase, Whiteboard: [Hixie-P4])

Attachments

(1 file)

Background applied to the root element, which then back-propagates to the canvas, are being aligned using the canvas origin as the starting point, instead of being drawn on the root element and then 'spilled' into the canvas. For example, a background drawn at "background-position: center center" on a root element of "height:100px; width:100px;" should be drawn bang in the center of the root element, and NOT in the center of the viewport. Note that according to CSS, the canvas is infinite anyway, so it *has* no origin. So there is no theoretically 'top left' for the canvas. ;-)
This bug broke out of bug 2285. Nominating for nsbeta3. This is a spec compliance issue. As currently implemented, we prevent web authors from creating valid documents that have well aligned backgrounds.
QA Contact: petersen → py8ieh=bugzilla
Is this really a regression? I don't think it ever worked... Regardless, I'm gonna fix it. Thanks for the testcases, Ian.
Status: NEW → ASSIGNED
Component: Layout → Style System
OS: Windows 2000 → All
Hardware: PC → All
Target Milestone: --- → M18
We used to not propagate the background to the canvas at all, and at that time the background was positioned correctly in the root element. So yeah, this particular aspect of the issue is a regression. ;-)
nsCSSRendering::PaintBackground always uses the frame passed in to it to determine the origin, except in the case where there is no scrolling view for the frame: if (!scrolledFrame) { // The viewport isn't scrollable, so use the root frame's view nsIPresShell* presShell; nsIFrame* rootFrame; aPresContext->GetShell(&presShell); presShell->GetRootFrame(&rootFrame); NS_RELEASE(presShell); rootFrame->GetView(aPresContext, (nsIView**)&viewportView); NS_ASSERTION(viewportView, "no viewport view"); viewportView->GetDimensions(&viewportArea.width, &viewportArea.height); } Maybe this logic can be extended to consider the canvas frame as well, and use the root element's bounds for the origin calculation in that case. _Sounds_ logical anyway...
[nsbeta3+].
Whiteboard: [nsbeta3+]
Priority: P3 → P2
Demoting to beta3- : this bug is not going to stop us from shipping the product. Resources being applied to fix this can be better put toward fixing some ship-stoppers elsewhere in layout. Sorry :( If an outside contributor can help with this, please feel free to jump in. I spent some time trying to massage nsCSSRendering::PaintBackground's API to allow another origin to be provided, but not concrete results so far.
Whiteboard: [nsbeta3+] → [nsbeta3-]
Target Milestone: M18 → Future
Ok. It's a pity, though, since it is pretty much the only remaining issue with our rendering of CSS backgrounds in HTML documents. :-)
Keywords: helpwanted
Yea, I agree. And the pities are lining up behind it... Maybe I'll get insomnia in the next few weeks and will fix it on my own time ;)
Summary: canvas background uses canvas origin not HTML origin → canvas background uses canvas origin not HTML origin [BG]
Keywords: nsbeta3mozilla1.0
Are you sure this is even a bug? I don't think the rules for background propagation are clearly defined by the spec, and some proposals for them may not make this a bug... (Or did I forget about some clarification.)
Whiteboard: [nsbeta3-] → [Hixie-P4] [nsbeta3-]
dbaron: The canvas has no origin. The viewport is not fixed. Aligning the background image with the original top left position of the viewport is arbitrary at best. Why would you _not_ want to do this? What if you have multiple viewports? Which do you pick as the origin? What does background-position: 4cm 33%; ...mean if the background-position is relative to the canvas?
Summary: canvas background uses canvas origin not HTML origin [BG] → canvas background uses canvas 'origin' not HTML origin [BG]
I added an argument to the |ComputeBackgroundAnchorPoint| helper function so that it can make the anchor be relative to one rect but offset from another. I then made the main PaintBackground code have three clearer logic paths when it tries to find the anchor point: 1. for fixed backgrounds, use the existing code to find the nearest scrolling ancestor. I coalesced two if statements into one and factored out some of the calls to optimise this a bit too. 2. for the canvas frame, find the first child frame of the canvas, assume it is the root element's frame, and find the anchor point relative to that frame's padding edge. 3. for everything else, just do what was done before. This passes the relevant tests at: http://www.hixie.ch/tests/adhoc/css/background/ ...modulo other pre-existing bugs. Looking for r=, sr= and check-in buddy please...
Assignee: attinasi → ian
Status: ASSIGNED → NEW
Keywords: helpwantedpatch
QA Contact: ian → dbaron
Whiteboard: [Hixie-P4] [nsbeta3-] → [Hixie-P4] [fixinhand]
Target Milestone: Future → mozilla0.9.4
Hixie: you solved more than 3 bugs and easily got the reviews for them ; you should have cvs write access and be your own check-in buddy ;-)
Nice one, Hixie! sr=attinasi - so, does this finally close the chapteron our body background image story for HTML?
Daniel: I have CVS access, I just need three superreviewers (one of which has to not work for AOL) to say it's ok for me to check in to SeamonkeyAll. Marc: I wish. :-)
dcone: Could you review this patch? Thanks! pavlov: you might want to take a quick look at the patch since it touches code you recently touched with the imglib1-removal stuff. Shouldn't be anything serious though.
r=dcone. Looks good, I am sure you have many more tests then me for this. Good work.
Checked in.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Whiteboard: [Hixie-P4] [fixinhand] → [Hixie-P4]
verified
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: