405659 - Gradient stroke fails to paint on shift+reload

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jonathan Watt [:jwatt]]

On the W3C test http://www.w3.org/Graphics/SVG/Test/20061213/htmlEmbedHarness/full-pservers-grad-11-b.html if I hold down shift and hit reload then the text "Gradient on text stroke" does not paint in the SVG. If I press just reload, it paints fine.

I've seen this sort of weirdness on and off in SVG for a while, so requesting blocking1.9.

Comment 1

[Robert Longson [:longsonr]]

Attachment: patch

I should have reverted this when I implemented fallback colours. Note how the implementation now matches HasFill.

Comment 2

[Jonathan Watt [:jwatt]]

Looks like you made this change to HasStroke already, then backed it out. I need to look closer at the history though.

Comment 3

[Robert Longson [:longsonr]]

I did make the change originally, now I want to back out the only line left from bug 354295. I should have removed this line as part of bug 264132 when I moved all of the other code in bug 354295 to layout/style. Arguably the HasStroke change was always wrong but it didn't start to be a problem till other changes to covered region processing landed.

The issue is that when the page first loads the URL cannot be resolved to a frame so HasStroke returns false. This results in the text being given no covered region. When the style system eventually updates the page because the URL is now resolvable it refreshes an empty region.

Comment 4

[Jonathan Watt [:jwatt]]

Comment on attachment 290483 [details] [diff] [review]
patch

Ah, I see. It would be nice to have a comment in the source explaining why we check for eStyleSVGPaintType_Server instead of NS_STATE_SVG_STROKE_PSERVER (same for fill).

Can you add a comment before the NS_STATE_SVG_STROKE_PSERVER define in nsSVGUtils.h saying something like "Indicates we have a valid paint server fetchable using GetProperty". (Same for fill.)

Comment 5

[Robert Longson [:longsonr]]

Attachment: address review comments

Comment 6

[Robert Longson [:longsonr]]

Low risk, a one line change to make HasStroke work like HasFill already does.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 290531 [details] [diff] [review]
address review comments

a=beltzner for drivers

Comment 8

[Robert Longson [:longsonr]]

checked in.