[MAGNOLIA-3846] dialogs: the dialogs sometimes don't show the fields/tabs in latest Safari (Lion) Created: 27/Sep/11  Updated: 30/Dec/11  Resolved: 30/Dec/11

Status: Closed
Project: Magnolia
Component/s: admininterface, gui
Affects Version/s: 4.4
Fix Version/s: 4.4.6, 4.5

Type: Bug Priority: Blocker
Reporter: Philipp Bärfuss Assignee: Federico Grilli
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Safari 5.1.1, 5.1.2 under both OS X Tiger and Lion


Attachments: PNG File wrong-size-dialog-control.png    
Issue Links:
relation
is related to MAGNOLIA-3849 With Firefox 7 and Firebug all dialog... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:

 Description   

workaround: resizing the dialog shows the fields/tab



 Comments   
Comment by Jan Haderka [ 27/Sep/11 ]

I get the same from time to time also in 10.6.8.

Comment by Boris Kraft [ 03/Nov/11 ]

This problem also existed on OSX before Lion, I think it has something to do with some webkit update. If seems to work without problems on FF.

Comment by Federico Grilli [ 06/Dec/11 ]

Just a pattern I seemed to notice, at least with Safari version 5.1.1 (going to test new 5.1.2 as soon as possible): you only need to resize dialogs manually once, that is the first time you get one showing the issue. Any subsequent opening will show no problem.

Comment by Federico Grilli [ 08/Dec/11 ]

After upgrading to latest Safari (5.1.2) the problem seems to have gone. The Safari 5.1.2 page at Apple http://support.apple.com/kb/DL1070 does not have detailed release notes but only a generic mention of improved stability. Will keep this open and try to test it again during the day to double-check that the issue has actually gone.

Comment by Federico Grilli [ 08/Dec/11 ]

Alas, the issue popped up again with Safari 5.1.2 too. The problem is that reproducing it is not so easy as it seems to happen randomly, then you resize the dialog and it does not manifest for a while. I also tried to look at the dialog source with Safari inspector when the issue shows up but then opening the inspector resizes the dialog and the problem is gone even before one can start to investigate.

Comment by Philipp Bärfuss [ 20/Dec/11 ]

Here is where you have to start looking:

  • info.magnolia.cms.gui.dialog.Dialog.drawHtmlPreSubsHead(Writer)

The call to window.resizeTo() might be to early (has to wait for the DOM being ready?

Comment by Samuel Schmitt [ 22/Dec/11 ]

No more an issue. Several tests on Lion/Leopard with Safari 5.1.1 / 5.1.2 and this issue never happened anymore.

Comment by Jan Haderka [ 22/Dec/11 ]

I disagree. According to the comment from Federico, issue occurs even on Safari 5.1.2 tho less frequently. I think the solution outlined by Philipp should still be applied.
If I remember correctly it occurred more often when there were many tabs open in Safari and in general high load on the system.

Comment by Samuel Schmitt [ 23/Dec/11 ]

According to Philipp's comment, we should call the resize method later, meaning when DOM is ready.
There are several options:

1. jQuery provides a method $(document).ready(), this method is bullet proof and working across all the browsers. But jQuery is not loaded with the dialogs.

2. we could develop our custom DomReady method, but I dont guarantee the same quality than the one from jQuery

3. We could call the method window.resizeTo later in the document, add the write in Dialog#drawHtmlPost*

Comment by Boris Kraft [ 23/Dec/11 ]

jquery in the dialogs sounds powerful
Since it's cached anyways, why not just use it?

Comment by Federico Grilli [ 28/Dec/11 ]

a screenshot of the problematic dialog along with Safari inspector showing a wrong height of 1px for the div holding the controls.

Comment by Federico Grilli [ 28/Dec/11 ]

So, the js function mgnlDialogResizeTabs() from dialog.js computes the height for the element which sometimes has wrong height 1px, e.g. <div id="mgnlControl_0_div" class="mgnlDialogTab" style="left: 8px; width: 782px; height: 1px; "> and sets it in the element in the style attribute. That height depends on another js function mgnlGetWindowSize() which returns an object holding the width and height of the containing dialog. It might be that sometimes when mgnlGetWindowSize() is called the wrong height is returned due to timing issues (DOM not ready) but in my debugging attempts I've never been able to see a suspect value being returned. One thing which might be related to the issue happens at info.magnolia.cms.gui.dialog.Dialog.drawHtmlPostSubsTabSet(Writer) line #255 where a call to out.write("mgnlDialogResizeTabs('" + id + "');") is set, even though we saw that that function does not take args. I'll fix that and see what happens.

Comment by Federico Grilli [ 28/Dec/11 ]

Another thing to notice is that the dialog, even when containing a 1px high tab, has always the correct height.

Comment by Federico Grilli [ 28/Dec/11 ]

Further debugging showed that

  • when things go fine mgnlDialogResizeTabs() is called twice: the first time, reckoned div height is 1px whilst
    the second call, triggered by eventHandlerOnResize, which in turn should be triggered by window.resizeTo(..), sets the correct height
  • when things go wrong, mgnlDialogResizeTabs() is called only once and returns 1px.
    So, as Philipp suspected, it looks like a timing issue, as sometimes window.resizeTo() is not called (possibly due to DOM not ready?).
Comment by Federico Grilli [ 28/Dec/11 ]

Latest findings: when the issue shows up mgnlGetWindowSize() returns a 100px height (the 1px value set as the div style attr is the result of subtracting from it some top and bottom margins). The weird thing is that the width returned by mgnlGetWindowSize() seems to be always correct. I even tried to delay the call to mgnlDialogResizeTabs() as late as possible (just before the end of the body) but to no avail. Personally I don't know if it is worth to keep on spending time trying to fix this. At this point it looks to me it is more a Safari glitch (all other browsers work fine and Safari versions prior to 5.1.1 seem to be fine too). Maybe with the next version of it the issue will be gone.

Comment by Federico Grilli [ 29/Dec/11 ]

Eventually I opted for introducing jQuery in dialogs and use its ready function to call dialog resizing functions only when DOM is ready. This in fact seems to do the trick for Safari. Minified jQuery adds only ~90 KB weight more to the dialogs and it's cached anyway. Being used in noConflict mode there should not arise conflicts with mgnl js own definition of $ sign.

Comment by Daniel Lipp [ 30/Dec/11 ]

We should not introduce commented out tests - the later we try to reactivate them them more time we'll have to spend. I'll take a look on how to make it run right now.

Generated at Mon Feb 12 03:50:14 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.