Opened 11 years ago

Closed 10 years ago

#9357 closed defect (fixed)

[patch] [cla] DataChart output is screwed/courrupted with htmlLabels: false in 1.4.X

Reported by: Jared Jurkiewicz Owned by: Jared Jurkiewicz
Priority: blocker Milestone: 1.4
Component: Charting Version: 1.3.1
Keywords: Cc: Mike Wilcox, Karl Tiedt
Blocked By: Blocking:

Description (last modified by Jared Jurkiewicz)

DataChart? output is screwed/corrupted with htmlLabels: false in 1.4.X

The same problem does not occur in 1.3.1.

Using a modified version of the DataChart? test produces it. Will attach shortly along with screenshot of damaged output:

Attachments (4)

test_DataChart.html (5.6 KB) - added by Jared Jurkiewicz 11 years ago.
Testcase
stock.json (1.2 KB) - added by Jared Jurkiewicz 11 years ago.
Testcase support file.
mangled.jpg (73.4 KB) - added by Jared Jurkiewicz 11 years ago.
Screenshot form FF2 showing mangling.
9357.patch (419 bytes) - added by liucougar 10 years ago.

Download all attachments as: .zip

Change History (30)

Changed 11 years ago by Jared Jurkiewicz

Attachment: test_DataChart.html added

Testcase

Changed 11 years ago by Jared Jurkiewicz

Attachment: stock.json added

Testcase support file.

comment:1 Changed 11 years ago by Jared Jurkiewicz

The same mangling occurs on IE7, so it's not SVG specific.

Changed 11 years ago by Jared Jurkiewicz

Attachment: mangled.jpg added

Screenshot form FF2 showing mangling.

comment:2 Changed 10 years ago by Jared Jurkiewicz

severity: criticalblocker

Raising severity. We should not ship 1.4 without this fixed.

comment:3 Changed 10 years ago by Jared Jurkiewicz

Description: modified (diff)
Summary: DataChart output is screwed/courrupted with htmlLabels: true in 1.4.XDataChart output is screwed/courrupted with htmlLabels: false in 1.4.X

comment:4 Changed 10 years ago by Mike Wilcox

Cc: Mike Wilcox added

This happened after the performance refactor of the charting labels soon after the 1.3 release. If someone gives me some clues as to what's broken and where to look, I can try to help.

comment:5 Changed 10 years ago by Karl Tiedt

Cc: Karl Tiedt added

This appears to be the problem I am running into... thanks Jared! Mike, I can add this tibit of data from my tests trying to track down this issue.

My charts appear extremely NARROW and ignore the CSS width's set on the chart's div however if I give my charts x axis a large min/max variance... the chart begins to render more and more normal... understanding of this is that its only taking into account the # of data items for each series * step sizes in determining the width of a chart... (speculation but seems pretty sound)

comment:6 Changed 10 years ago by Tom Trenka

comment:7 Changed 10 years ago by alex

Owner: changed from Eugene Lazutkin to alex

comment:8 Changed 10 years ago by Karl Tiedt

Missplaced blame!

http://bugs.dojotoolkit.org/changeset/17597

Alex's code does *not* break this test :)

Liucougars however.... *sigh*

comment:9 Changed 10 years ago by liucougar

if you revert [17597], does it work?

the original code fixed in [17597] actually returns nothing (because width/height is not defined at all)

comment:10 Changed 10 years ago by Karl Tiedt

comment:11 Changed 10 years ago by liucougar

according to IE getBoundingClientRect MSDN doc, width/height is not in the returned object (TextRectangle? object)

according to FF getBoundingClientRect doc, width/height is added to FF 3.5 (not in 3.0 either)

could you try to modify _getTextBox in gfx._base to just do return { w: undefined, h: undefined };

does that fix your test?

comment:12 Changed 10 years ago by Karl Tiedt

results in same output prior to your patch. (same url)

comment:13 Changed 10 years ago by Jared Jurkiewicz

Any status on this? It's still ugly as crap in 1.4 with htmlLabels: false.

comment:14 Changed 10 years ago by Jared Jurkiewicz

Owner: changed from alex to liucougar

I confirm if I revert [17597], the problem goes away. It's definitely related to that checkin.

Assigning over to Liu to look further at, since it is his change that is breaking it.

Changed 10 years ago by liucougar

Attachment: 9357.patch added

comment:15 Changed 10 years ago by liucougar

Owner: changed from liucougar to Eugene Lazutkin

[17597] is correct, afaict, something is broken in dojox.charting when htmlLabel=false, here is a patch to fix it, but I don't know why it fixes this bug, pass it to elazutkin

comment:16 in reply to:  15 Changed 10 years ago by Karl Tiedt

Replying to liucougar:

[17597] is correct, afaict, something is broken in dojox.charting when htmlLabel=false, here is a patch to fix it, but I don't know why it fixes this bug, pass it to elazutkin

Correct me if I'm wrong, but if a bug is fixed, shouldn't the underlying code that uses it be checked for a) functionality and b) corrections needed because of the bug that was fixed?

If we keep passing this around, its never going to get fixed... [17597] should have included any changes for GFX based code (particularly for something like Charting that is one of our *big* dojox attractions)... just my 2cents...

comment:17 Changed 10 years ago by bill

Summary: DataChart output is screwed/courrupted with htmlLabels: false in 1.4.X[patch] [cla] DataChart output is screwed/courrupted with htmlLabels: false in 1.4.X

comment:18 Changed 10 years ago by Jared Jurkiewicz

Priority: normalhigh

Followup. Any word on this? The corruption is really, really bad/ugly.

comment:19 Changed 10 years ago by Jared Jurkiewicz

We really need to get this fixed for 1.4. :-(

comment:20 Changed 10 years ago by Jared Jurkiewicz

Priority: highhighest

comment:21 Changed 10 years ago by Eugene Lazutkin

Milestone: tbd1.4

comment:22 Changed 10 years ago by Eugene Lazutkin

(In [20320]) dojo.charting: fixing corrupted labels non-HTML labels, unrelated pie label positioning issue (not centered), and some minor performance corrections, !strict, refs #9357.

comment:23 in reply to:  22 Changed 10 years ago by Eugene Lazutkin

Owner: changed from Eugene Lazutkin to Jared Jurkiewicz

For the record: liucougar's patch was right (there was one more place where it should be applied), all effects were related to the original speed-up patch (it looks like a simple oversight).

!getBoundingClientRect returns left, right, top, bottom values following the original Microsoft spec, but newer implementations (see http://www.w3.org/TR/cssom-view/#clientrect for details) return width and height as well. That's why that code worked on some browsers and didn't work on others.

I fixed another bug introduced by the speed-up patch: wrongly positioned pie chart labels.

Jared, please re-test with whatever code you have to confirm that it fixed all problems.

comment:24 Changed 10 years ago by Eugene Lazutkin

The original ticket: #9011.

comment:25 Changed 10 years ago by Eugene Lazutkin

The related ticket: #9065.

comment:26 Changed 10 years ago by Jared Jurkiewicz

Resolution: fixed
Status: newclosed

The fix works nicely now! No more wonky charts with htmlLabels: false.

Export SVG works very nicely with charting using such labels.

Closing.

Note: See TracTickets for help on using tickets.