Problems with platform windows in IE9/10/11

cappuccino / cappuccino

Web Application Framework in JavaScript and Objective-J

https://cappuccino.dev/

GNU Lesser General Public License v2.1

2.21k stars 333 forks source link

Problems with platform windows in IE9/10/11 #2216

Closed nigelgoodship closed 7 years ago

nigelgoodship commented 9 years ago

In IE9, 10 & 11, new platform windows have no content when opened and after they're closed the main window throws exceptions when the mouse is moved over it.

This behaviour can be seen in the PlatformWindows manual test. A manifestation of the problems after the new platform window is closed is that the menus in the main window are no longer drawn properly. The test runs fine in Chrome, Firefox and Safari.

cappbot commented 9 years ago

Milestone: Someday. Label: #new. What's next? A reviewer should examine this issue.

ahankinson commented 9 years ago

-#new +bug +IE

cappbot commented 9 years ago

Milestone: Someday. Labels: IE, bug. What's next? A reviewer should examine this issue.

RonchettiAssociati commented 9 years ago

Is there a workaround for this?

In my application the exception is thrown at the makeKeyAndOrderFront: method.

The IE debugger is not very helpful in tracing this further.

Same error happens with the orderFront method.

RonchettiAssociati commented 9 years ago

AFAIU the point where IE breaks is in the layerAtLevel:create: method of CPPlatformWindow+DOM

and specifically the lines layer._DOMElement.style.zIndex = aLevel + 1; // adding one avoids negative zIndices. These have been causing issues in Chrome _DOMBodyElement.appendChild(layer._DOMElement);

maybe the fix for Chrome has broken things for IE?

daboe01 commented 9 years ago

@RonchettiAssociati does the issue go away when you change the line to layer._DOMElement.style.zIndex = aLevel (remove the +1)?

ahankinson commented 9 years ago

I'm checking this right now -- will update.

ahankinson commented 9 years ago

I seem to be getting these error messages:

SCRIPT5007: Unable to get property 'style' of undefined or null reference 
index-debug.html, line 353 character 9
SCRIPT16389: Unspecified error. 
index-debug.html, line 799 character 9

@RonchettiAssociati Are these the same errors you're getting?

daboe01 commented 9 years ago

hi @ahankinson, thanks for testing! for that matter, could you try the following patch?: if(layer && layer._DOMElement && layer._DOMElement.style) layer._DOMElement.style.zIndex = aLevel + 1

ahankinson commented 9 years ago

Yeah, that doesn't seem to solve the problem; at least not as far as I can tell. My IE-fu is pretty bad, and the ahem "quality" of the developers tools doesn't help.

ahankinson commented 9 years ago

I'm using IE10; I'll upgrade to IE11 and see if those dev tools are a bit better.

ahankinson commented 9 years ago

+#accepted +#needs-info

RonchettiAssociati commented 9 years ago

Hi guys - I am on IE11 emulating Windows 8.1 via Parallels.

NO - removing the +1 does not solve NO - the if(layer && layer._DOMElement && layer._DOMElement.style) does not solve either.

My IE console says SCRIPT5022: Exception thrown and not caught File RoAGdo, line 774, column 9

I am trying to get a break on exception at the line mentioned, but have not been able to do so (I did it couple of hours ago, but now seem unable to get there again).

The developer here (me) is even worse than the debugging tools.

On Nov 18, 2014, at 3:55 PM, Andrew Hankinson notifications@github.com wrote:

+#accepted +#needs-info

— Reply to this email directly or view it on GitHub https://github.com/cappuccino/cappuccino/issues/2216#issuecomment-63482344.

cappbot commented 9 years ago

Milestone: Someday. Labels: #accepted, IE, bug. What's next? A reviewer should examine this issue.

RonchettiAssociati commented 9 years ago

Hi,

It seems to me that it fails on the last statement of the layerAtLevel:create: method of CPPlatformWindow+DOM method, ie:

        _DOMBodyElement.appendChild(layer._DOMElement);

I am now able to look at the objects.

_DOMBodyElement.outerHTML looks like:

<body style="cursor: default; background-color: transparent;"><input class="cpdontremove" style="position: absolute; z-index: -1000; opacity: 0;"><div class="cpdontremove" style="left: 0px; top: 0px; width: 100%; height: 100%; display: none; position: absolute; z-index: 999;"></div><div class="cpdontremove" style="width: 60px; height: 60px; overflow: scroll; visibility: hidden; position: absolute; z-index: 999; opacity: 0;"><div style="width: 400px; height: 400px;"></div></div></body>

and layer.DOMElement.outerHTML looks like:

<div style="left: 0px; top: 0px; width: 1px; height: 1px; position: absolute; z-index: 0;"></div>

That seems legit to me, maybe apart from the z-index, as we already have a

with z-index:999

But I am sorry I do not really understand html.

Cheers.

daboe01 commented 9 years ago

@RonchettiAssociati i do not think that the errors are related to the value of z-index. it seems rather that we are accessing/writing a property that is undefined in IE.

ahankinson commented 9 years ago

I think I've tracked it down. It's failing on _setCSSStyleForInputElement on CPTextField.j, where a call to [self _inputElement] returns null. The error is shown in the following screenshot:

cappscreenshot

I believe this can be traced back to this commit:

https://github.com/cappuccino/cappuccino/commit/0c7b2ae7023ba18a59f75cbf868f28e0d53917f9

Where the specific case of CPTextFieldDOMInputElement is no longer checked. If so, @primalmotion may be able to lend us a hand and let us know why he changed this check.

daboe01 commented 9 years ago

good catch :-)

RonchettiAssociati commented 9 years ago

Wow, that’s great. Thank you.

On Nov 24, 2014, at 8:02 AM, daboe01 notifications@github.com wrote:

goot catch :-)

— Reply to this email directly or view it on GitHub https://github.com/cappuccino/cappuccino/issues/2216#issuecomment-64157332.

ahankinson commented 9 years ago

I spent an hour or so with this issue today and can report significant progress, but no solution (but we're close).

It isn't where I suspected. The CPTextField code checks out ok.

However, I created a new PlatformWindow project (with the new index-debug.html) and copied over the AppController code from the original one. Then I turned on the following debugging options in index-debug.html:

objj_msgSend_decorate(objj_backtrace_decorator);
objj_msgSend_decorate(objj_supress_exceptions_decorator)
objj_typecheck_prints_backtrace = true;

What I got was this:

HTML1300: Navigation occurred.
File: index-debug.html
2014-11-24 12:24:34.120 Cappuccino [warn]: Exception HierarchyRequestError in [<CPPlatformWindow 0x009f37> layerAtLevel:create:]
2014-11-24 12:24:34.145 Cappuccino [warn]: [<CPRunLoop 0x000848> limitDateForMode:]
2014-11-24 12:24:34.151 Cappuccino [warn]: [<CPTimer 0x009f39> fire]
2014-11-24 12:24:34.155 Cappuccino [warn]: [<CPWindow 0x009e4f> orderFront:]
2014-11-24 12:24:34.170 Cappuccino [warn]: [<CPWindow 0x009e4f> orderWindow:relativeTo:]
2014-11-24 12:24:34.175 Cappuccino [warn]: [<CPWindow 0x009e4f> _orderFront]
2014-11-24 12:24:34.176 Cappuccino [warn]: [<CPPlatformWindow 0x009f37> order:window:relativeTo:]
2014-11-24 12:24:34.180 Cappuccino [warn]: [<CPPlatformWindow 0x009f37> layerAtLevel:create:]
2014-11-24 12:24:34.201 Cappuccino [warn]: Exception HierarchyRequestError in [<CPPlatformWindow 0x009f37> layerAtLevel:create:]

Which led me to:

http://stackoverflow.com/questions/23097024/internet-explorer-throws-script5022-hierarchyrequesterror-when-trying-to-append

So it looks like there's a problem with IE when moving an element from a Capp Window to a platform window. I'm not sure where in the code this is, and don't have any more time today to look into it, but I hope this helps.

boucher commented 9 years ago

Can I just ask, does anyone know if this ever worked? I know for a fact that it did not when we shipped Platform windows, because they were intended only for native-host. (At the time we determined it wasn't possible to make them work in IE, but I can't recall why)

On Mon, Nov 24, 2014 at 10:07 AM, Andrew Hankinson <notifications@github.com

wrote:

I spent an hour or so with this issue today and can report significant progress, but no solution (but we're close).

It isn't where I suspected. The CPTextField code checks out ok.

However, I created a new PlatformWindow project (with the new index-debug.html) and copied over the AppController code from the original one. Then I turned on the following debugging options in index-debug.html:

objj_msgSend_decorate(objj_backtrace_decorator); objj_msgSend_decorate(objj_supress_exceptions_decorator) objj_typecheck_prints_backtrace = true;

What I got was this:

HTML1300: Navigation occurred. File: index-debug.html 2014-11-24 12:24:34.120 Cappuccino [warn]: Exception HierarchyRequestError in [<CPPlatformWindow 0x009f37> layerAtLevel:create:] 2014-11-24 12:24:34.145 Cappuccino [warn]: [<CPRunLoop 0x000848> limitDateForMode:] 2014-11-24 12:24:34.151 Cappuccino [warn]: [<CPTimer 0x009f39> fire] 2014-11-24 12:24:34.155 Cappuccino [warn]: [<CPWindow 0x009e4f> orderFront:] 2014-11-24 12:24:34.170 Cappuccino [warn]: [<CPWindow 0x009e4f> orderWindow:relativeTo:] 2014-11-24 12:24:34.175 Cappuccino [warn]: [<CPWindow 0x009e4f> _orderFront] 2014-11-24 12:24:34.176 Cappuccino [warn]: [<CPPlatformWindow 0x009f37> order:window:relativeTo:] 2014-11-24 12:24:34.180 Cappuccino [warn]: [<CPPlatformWindow 0x009f37> layerAtLevel:create:] 2014-11-24 12:24:34.201 Cappuccino [warn]: Exception HierarchyRequestError in [<CPPlatformWindow 0x009f37> layerAtLevel:create:]

Which led me to:

http://stackoverflow.com/questions/23097024/internet-explorer-throws-script5022-hierarchyrequesterror-when-trying-to-append

So it looks like there's a problem with IE when moving an element from a Capp Window to a platform window. I'm not sure where in the code this is, and don't have any more time today to look into it, but I hope this helps.

— Reply to this email directly or view it on GitHub https://github.com/cappuccino/cappuccino/issues/2216#issuecomment-64236318 .

RonchettiAssociati commented 9 years ago

Cappuccino assigns the CPWindow to the CPPlatformWindow via the setPlatformWindow: method. I understand that the window is "created within" the original CPPlatformWindow and then reassigned.

The thread that Andrew mentions above and this other preceding one http://stackoverflow.com/questions/23071968/jquery-throws-script5022-hierarchyrequesterror-when-attempted-to-append-a-jquer?lq=1 suggest that in IE you should do this by setting the innerHTML element.

Unfortunately I have not found the code in CPWindow that actually appends the DOM element to the platform window and therefore am not able to check how this is done in reality. I would appreciate if someone can point me in the right direction.

boucher commented 9 years ago

You'll probably want to dig around in this folder for whatever you are looking for: https://github.com/cappuccino/cappuccino/tree/master/AppKit/Platform/DOM

On Tue, Nov 25, 2014 at 8:47 AM, RonchettiAssociati < notifications@github.com> wrote:

Cappuccino assigns the CPWindow to the CPPlatformWindow via the setPlatformWindow: method. I understand that the window is "created within" the original CPPlatformWindow and then reassigned.

The thread that Andrew mentions above and this other preceding one http://stackoverflow.com/questions/23071968/jquery-throws-script5022-hierarchyrequesterror-when-attempted-to-append-a-jquer?lq=1 suggest that in IE you should do this by setting the innerHTML element.

Unfortunately I have not found the code in CPWindow that actually appends the DOM element to the platform window and therefore am not able to check how this is done in reality. I would appreciate if someone can point me in the right direction.

— Reply to this email directly or view it on GitHub https://github.com/cappuccino/cappuccino/issues/2216#issuecomment-64430911 .

RonchettiAssociati commented 9 years ago

Thanks Ross.

There is indeed in https://github.com/cappuccino/cappuccino/blob/master/AppKit/Platform/DOM/CPDOMDisplayServer.h a line that reads like:

#define CPDOMDisplayServerAppendChild(aParentElement, aChildElement) aParentElement.appendChild(aChildElement)

Maybe in it we could change the simple appendChild call with (adapted form NoGray's suggestion on stackoverflow):

try {
    aParentElement.appendChild(aChildElement); 
}
catch (e){
    if (aChildElement.outerHTML) {
        aParentElement.innerHTML = aChildElement.outerHTML;
    }
    else {
            console.log('not working');
    }
}

Not sure how to do it, though. Can I simply put the new #define right before my stuff?

Thanks for suggestions (and pardon my incompetence).

boucher commented 9 years ago

Well, it's quite likely more complicated than that, since the owner of the reference to aChildElement likely has references to many of its sub elements. The actual reference matters, and will be blown away by the conversion from and to HTML. It's likely a major restructuring would be needed to make that work.

On Tue, Nov 25, 2014 at 9:24 AM, RonchettiAssociati < notifications@github.com> wrote:

Thanks Ross.

There is indeed in https://github.com/cappuccino/cappuccino/blob/master/AppKit/Platform/DOM/CPDOMDisplayServer.h a line that reads like:

define CPDOMDisplayServerAppendChild(aParentElement, aChildElement) aParentElement.appendChild(aChildElement)

Maybe in it we could change the simple appendChild call with (adapted form NoGray's suggestion on stackoverflow):

try { aParentElement.appendChild(aChildElement); } catch (e){ if (aChildElement.outerHTML) { aParentElement.innerHTML = aChildElement.outerHTML; } else { console.log('not working'); } }

Not sure how to do it, though. Can I simply put the new #define right before my stuff?

Thanks for suggestions (and pardon my incompetence).

— Reply to this email directly or view it on GitHub https://github.com/cappuccino/cappuccino/issues/2216#issuecomment-64437350 .

daboe01 commented 7 years ago

i tend to close this one. IE is already depreciated by MS. By the time of capp 1.0 it will be most likely obsoleted. @aparajita what do you think?

aparajita commented 7 years ago

IE is not deprecated, but versions before 12 are unsupported (and have negligible market share). Edge doesn't run on Windows 7, and there will be a large user base on Windows 7 for a while. So we will support IE 12 for a while.

daboe01 commented 7 years ago

IE 12 is "Edge"?

daboe01 commented 7 years ago

I do not have Win10. Can someone else please check this issue on IE12 please?

aparajita commented 7 years ago

IE 12 is "Edge"?

No, it’s a completely new browser that only runs on Windows 10. IE 12 runs on Windows 7.

Many thanks,

Aparajita

RonchettiAssociati commented 7 years ago

Platform windows run just fine with Edge on Windows10. I don't know about IE12 and Windows7, sorry.

On 22 giu 2016, at 07:27, daboe01 notifications@github.com wrote:

I do not have Win10. Can someone else please check this issue on IE12 please?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

daboe01 commented 7 years ago

@aparajita there is no IE12 in the microsoft download center. The last version there is IE11. given bouchers comment that fixing this is next to impossible i vote for closing as wont-fix.

nigelgoodship commented 7 years ago

As I raised the issue, I feel I should comment...

I, too, can't find an IE12 for Windows 7. I would be more than happy to test this issue on IE12 if I could find it. I've found references to IE12, but they are all 1 to 2 years old and seem to suggest it was a working title for a branch of the Trident rendering engine that has now become Edge. But I'm far from being an expert or even vaguely knowledgable about Microsoft's products, so this could easily be wrong.

Most of our customers who mainly use IE have agreed to also use Chrome for the sake of running our Cappuccino apps, so they do have this means of avoiding the problem in IE. On that basis, I can't really argue too strongly in favour of still trying to find a fix for this issue for IE11. Peoples' time would probably be better spent on other things within the Cappuccino project. I'm happy to leave the decision on whether to close this issue as won't-fix to the core developers.

aparajita commented 7 years ago

My mistake, IE 11 is the last version on Windows 7.

In any case, IE’s market share is tiny now, so we shouldn’t waste our time on it. Go ahead and close it.

daboe01 commented 7 years ago

thank you all :-) +#wont-fix

cappbot commented 7 years ago

Milestone: Someday. Labels: #wont-fix, IE, bug. What's next? A reviewer or core team member has decided against acting upon this issue.

cappbot commented 7 years ago

Milestone: Someday. Labels: #wont-fix, IE, bug. What's next? A reviewer or core team member has decided against acting upon this issue.