smas1 / geoext-viewer

Automatically exported from code.google.com/p/geoext-viewer
GNU General Public License v3.0
0 stars 0 forks source link

OLeditor - regular polygon selection #382

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. start editorbasics demo
2. select 'draw regular polygon'
3. select a non triangle entry from the listbox
4. in all cases a triangle is drawn

What is the expected output? What do you see instead?

the selected polygon should be drawn

Original issue reported on code.google.com by wolfram.winter on 26 May 2014 at 12:32

GoogleCodeExporter commented 9 years ago
Error code when changing the listbox entry from 'triangle' to any other entry:
Meldung: 'target.value' ist Null oder kein Objekt
Zeile: 84
Zeichen: 13
Code: 0
URI: 
http://172.28.39.217/LIB/heron-trunk/ux/oleditor/ole/client/lib/Editor/Control/D
rawRegular.js

Original comment by wolfram.winter on 5 Jun 2014 at 8:44

GoogleCodeExporter commented 9 years ago
Hi Just - it's an IE8 DOM problem - please have a look at the new 
'DrawRegular.js' - I've tested it with IE8 and FF30/31 - it works for me - can 
you please integrate the changed file in the ole project? Thanks!

Original comment by wolfram.winter on 28 Jul 2014 at 9:43

Attachments:

GoogleCodeExporter commented 9 years ago
Ok, I've had a look. Tricky problem. I don't have IE8 at hand. Offcourse I can 
always integrate in OLE. First step is creating an issue there: 
https://github.com/geops/ole/issues/35

Questions/suggestions on your solution, the lines 87-88:

this.handler.setOptions({sides: 
parseInt(document.getElementById('oleCADToolsDialogSides').value)});
this.handler.setOptions({irregular: 
document.getElementById('oleCADToolsDialogIrregular').checked});

- is setting the 'checked' field really required (line 88)?
- also in original: the default of 'true' is never set in the handler options
- if required multiple options can be set in one call to setOptions({sides: .., 
irregular: ...});
- IMO it would be simpler to use values directly from the variable declared. 
Like
this.handler.setOptions({sides: parseInt(sidesSelect.value)}); (as in line 90). 
This will work as the var is within the JavaScript Closure. We could do a 
similar thing for 'checked'.

I have attached a slightly I think improved version though was not able to 
check with IE8.

Original comment by jus...@gmail.com on 5 Aug 2014 at 12:46

Attachments:

GoogleCodeExporter commented 9 years ago
Hi Just - 
1.) the line
this.handler.setOptions({sides: parseInt(sidesSelect.value)});
works with IE8.
2.) I've added line 63 and line 88 for intialising the checkbox - if missing, I 
can reproduce the following behavior: start OLE, select the form tool, draw 
triangle, check iregular box, draw triangle again => error: its not drawn 
irregular.

Original comment by wolfram.winter on 6 Aug 2014 at 3:17

GoogleCodeExporter commented 9 years ago
I poorly must correct 1.) from before: your changes diddn't work for IE8... - 
I'll do some tests next monday...

Original comment by wolfram.winter on 6 Aug 2014 at 3:29

GoogleCodeExporter commented 9 years ago
Ok, their were two things wrong from the beginning: 

- IE needs special setting for checkbox default value before DOM insert: 
defaultChecked = true. This is also used at other places in the OLE code. 
- the value for the selected Option from the Select box must be done via the 
selectedIndex, like sidesSelect.options[sidesSelect.selectedIndex].value

This is reflected in the newly attached file here.

Original comment by jus...@gmail.com on 7 Aug 2014 at 1:11

Attachments:

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Hi Just - using your last changes with IE8 I found this strange behaviour: if 
you change the listbox entry or the checkbox state the irregular state of 
drawing isn't changed if your starting click of the form is INSIDE of any other 
just painted form. If the starting click doesn't touch an old form everything 
is ok.

Original comment by wolfram.winter on 11 Aug 2014 at 10:24

GoogleCodeExporter commented 9 years ago
Hi Wolfram, Ok, my changes were also in line with other uses of checkboxes en 
listboxes in OLEditor. Also I am testing in IE8 emulation Win7 and found no 
problems. But ok, you have deleted your comment #7, so it appearantly worked 
for you. Good to hear! 

I find it hard to follow your issue in comment #8. Maybe you can provide a 
scenario with steps just like above "What steps will reproduce the problem?" ?

Original comment by jus...@gmail.com on 11 Aug 2014 at 12:43

GoogleCodeExporter commented 9 years ago
Ok - I deleted comment #7, because I realized that the changes you made are ok 
(the comments in line 67-68 and 98-100 in DrawRegular.js could be deleted) and 
that the handling errors in IE8 are done by clicking on existing geometries - 
so here is the description of the remaining problem:
- start the editorbasic example
- select the regular polygon
- draw a triangle 
- change the irregular checkbox
- draw a triangle with the starting point INSIDE of the first triangle
=> result in IE8: the irregular status of the second triangle is not changed
- if you draw the second triangle with a starting point OUTSIDE of the first 
triangle, everything is ok
In FF31 this error doesn't appear...

Original comment by wolfram.winter on 12 Aug 2014 at 8:38

GoogleCodeExporter commented 9 years ago
Ok, first could reproduce #10 in IE8. This was using and including the separate 
DrawRegular.js file in index.html. Very hard to get grips on that problem. But 
after having integrated  DrawRegular.js into OLEditor GitHub and rebuilt 
ole.min.js the problem is gone!
So try: http://lib.heron-mc.org/heron/latest/examples/editorbasics . In my IE8 
both the original issue (always triangle drawn) and problem from #10 are solved.

New: with IE in script debug mode I always get an error from Map.destroy() 
(null reference for a DrawControl) when unloading the editorbasics page, even 
when not having drawned anything. This is similar if not the same as this OL 
bug: http://trac.osgeo.org/openlayers/ticket/2578, which has never been fixed 
in the way suggested with the patch there. Will open separate Heron and 
OpenLayers issue (OL should check for undefined controls) for this.

Original comment by jus...@gmail.com on 12 Aug 2014 at 11:01

GoogleCodeExporter commented 9 years ago
Hi Just - the original issue is solved. The problem from #10 is not solved - 
using the editorbasics demo and our test app with the new ole.imin.js did not 
work.

Original comment by wolfram.winter on 12 Aug 2014 at 2:17

GoogleCodeExporter commented 9 years ago
Hi Wolfram, Ok, strange for #10. What I can think of:

- I use IE11 (Edge) on Windows7 with IE8 emulation via Developer Tools
- does the problem occur with you on IE9 and up?

Original comment by jus...@gmail.com on 12 Aug 2014 at 3:04

GoogleCodeExporter commented 9 years ago
Hi Just - I've found a win7 system with IE9 in our office: your editorbasic 
demo runs without any #10 problem - only IE8 shows this side effect - STRANGE!!!

Original comment by wolfram.winter on 14 Aug 2014 at 8:55

GoogleCodeExporter commented 9 years ago
Well, not really strange, specifically IE8 has many other problems :-(.

- XML parsing XMLDOM rendering in particular NameSpace issues
- CSS3 support (background-size)
See https://code.google.com/p/geoext-viewer/issues/detail?id=324

I am not a Windows users but from what I read is IE8 the last IE on Windows XP 
and XP is EOL, so future versions like Win7 have IE9 or up (I have IE11).  Read 
also : http://theie8countdown.com. IE8 is the modern IE6. I think in our case 
it is a minor hick-up and very hard (hours) to fix for an almost obsolete 
browser and quite a rare scenario (drawing polygons within polygons).

I propose to close this issue and move the remaining IE8 problem in a new 
"drawing polygons within polygons in IE8 issue" Ok?

Original comment by jus...@gmail.com on 14 Aug 2014 at 9:26

GoogleCodeExporter commented 9 years ago
Hi Just - you are right - we have about 200 XP users out there, but the number 
will decrease (soon). I agree with you to close this issue - thanks for your 
support!

Original comment by wolfram.winter on 14 Aug 2014 at 12:27

GoogleCodeExporter commented 9 years ago
Closing and opened issue 401 for problem under #10.

Original comment by jus...@gmail.com on 15 Aug 2014 at 1:20