Opened 5 years ago

Last modified 20 months ago

#18228 assigned defect

[patch][test] dijit BackgroundIframe does not cover scrolled menu popup

Reported by: Ronald Pijnacker Owned by:
Priority: undecided Milestone: 1.14
Component: Dijit Version: 1.9.3
Keywords: Cc:
Blocked By: Blocking:

Description

When opening a popup containing a popup menu with a scroll-bar, part of the background can seep through. Also, when the content is scrolled, the background seeps through the part that was not visible before. See attached screenshots.

The cause of this problem is that the BackgroundIframe? that is attached to the popup wrapper does not cover the entire content of the wrapped node, nor does it cover the scrollbar.

Adding a second wrapper to handle the scrolling leaves the attached BackgroundIframe? inplace and makes sure it also covers the scrollbar. Please look at attached diff for the solution.

Attachments (5)

popup-wrong.png (26.2 KB) - added by Ronald Pijnacker 5 years ago.
Screenshot of wrong situation
popup-correct.png (11.9 KB) - added by Ronald Pijnacker 5 years ago.
Screenshot of corrected situation
popup.diff (1.5 KB) - added by Ronald Pijnacker 5 years ago.
diff of fix
popup_bug.html (2.4 KB) - added by Ronald Pijnacker 5 years ago.
Minimal reproduction
popup.js.patch (1.3 KB) - added by Michael Schall 21 months ago.
Patch to keep border and scrollbars showing when menu is over pdf in IE

Download all attachments as: .zip

Change History (16)

Changed 5 years ago by Ronald Pijnacker

Attachment: popup-wrong.png added

Screenshot of wrong situation

Changed 5 years ago by Ronald Pijnacker

Attachment: popup-correct.png added

Screenshot of corrected situation

Changed 5 years ago by Ronald Pijnacker

Attachment: popup.diff added

diff of fix

comment:1 Changed 5 years ago by bill

Owner: set to Ronald Pijnacker
Status: newpending

OK, please attach a minimal test case to reproduce this. Thanks!

Changed 5 years ago by Ronald Pijnacker

Attachment: popup_bug.html added

Minimal reproduction

comment:2 Changed 5 years ago by Ronald Pijnacker

Status: pendingnew

Attachment (popup_bug.html) added by ticket reporter.

comment:3 Changed 5 years ago by Ronald Pijnacker

Added minimal test case. Please note this only works in IE (because of the use of an activeX object).

comment:4 Changed 4 years ago by bill

Owner: Ronald Pijnacker deleted
Status: newassigned

comment:5 Changed 3 years ago by dylan

Milestone: tbd1.11
Summary: dijit BackgroundIframe does not cover scrolled menu popup[patch][test] dijit BackgroundIframe does not cover scrolled menu popup

Candidate for 1.11 or 1.12?

comment:6 Changed 3 years ago by dylan

Milestone: 1.111.12

Ok, after massive triage, ended up with about 80 tickets for 1.11 and 400 or so for 1.12. That's a bit unrealistic, so first I changed all 1.12 to 1.13 (with the plan to move some forward to the new 1.12. Now, I'm moving some of the 1.11 tickets that are less likely to get done this month without help to 1.11. Feel free to help out in January if you want to see this ticket land in 1.11.

comment:7 Changed 3 years ago by dylan

Milestone: 1.121.13

Ticket planning... move current 1.12 tickets out to 1.13 that likely won't get fixed in 1.12.

comment:8 Changed 21 months ago by Michael Schall

Here is a patch that will keep the border of the menu as well as the scrollbars.

Changed 21 months ago by Michael Schall

Attachment: popup.js.patch added

Patch to keep border and scrollbars showing when menu is over pdf in IE

comment:9 Changed 21 months ago by Michael Schall

Updated patch with additional fix for resized menus

  • popup.js

     
    145145                                        role: "region",
    146146                                        "aria-label": widget["aria-label"] || widget.label || widget.name || widget.id
    147147                                }, widget.ownerDocumentBody);
    148                                 wrapper.appendChild(node);
     148                                wrapper.borderWrapper = domConstruct.create("div", null, wrapper);
     149                                wrapper.scrollerWrapper = domConstruct.create("div", null, wrapper.borderWrapper);
     150                                wrapper.scrollerWrapper.appendChild(node);
    149151
    150152                                var s = node.style;
    151153                                s.display = "";
     
    215217
    216218                        domStyle.set(wrapper, {
    217219                                display: "none",
     220                        });
     221                        domStyle.set(wrapper.borderWrapper, {
     222                                border: ""                              // Open() may have moved border from popup to wrapper.
     223                        });
     224                        domStyle.set(wrapper.scrollerWrapper, {
    218225                                height: "auto",                 // Open() may have limited the height to fit in the viewport,
    219226                                overflowY: "visible",   // and set overflowY to "auto".
    220                                 border: ""                              // Open() may have moved border from popup to wrapper.
    221227                        });
    222228
    223229                        // Open() may have moved border from popup to wrapper.  Move it back.
     
    292298                                // and domStyle.get(node, "borderColor") etc. doesn't work on FF, so need to use fully qualified names.
    293299                                var cs = domStyle.getComputedStyle(node),
    294300                                        borderStyle = cs.borderLeftWidth + " " + cs.borderLeftStyle + " " + cs.borderLeftColor;
    295                                 domStyle.set(wrapper, {
     301                                domStyle.set(wrapper.borderWrapper, {
     302                                        border: borderStyle     // so scrollbar is inside border and border are inside the backgroundIframe
     303                                });
     304                                domStyle.set(wrapper.scrollerWrapper, {
    296305                                        overflowY: "scroll",
    297                                         height: maxHeight + "px",
    298                                         border: borderStyle     // so scrollbar is inside border
     306                                        height: maxHeight + "px"
    299307                                });
    300308                                node._originalStyle = node.style.cssText;
    301309                                node.style.border = "none";

comment:10 Changed 21 months ago by dylan

schallm, any chance you could raise this as a pull request in GitHub?? If you've not done much in the way of pull requests, we have some guidelines at https://github.com/dojo/dojo/blob/master/CONTRIBUTING.md , or feel free to ask me for help.

comment:11 Changed 20 months ago by dylan

Milestone: 1.131.14
Note: See TracTickets for help on using tickets.