Closed Bug 289973 Opened 15 years ago Closed 15 years ago

since 20050331:[Mac] White space shows up in XUL menu after redrawing

Categories

(Core Graveyard :: Widget: Mac, defect, P2, major)

PowerPC
macOS

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.8beta3

People

(Reporter: firefox, Assigned: sfraser_bugs)

References

Details

(Keywords: regression, Whiteboard: [no l10n impact] needs reduced testcase)

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050330 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050330 Firefox/1.0+

Open in Tabs will show an empty white spot on a second level folder after
selecting.  Selecting the empty white spot will still perform the Open in Tabs
function, but it may be confusing for some users.  This has nothing to do with
Live Bookmarks, and as far as I know, occurs only on Macs.  This has been
"broken" since 3/31's build.

Reproducible: Always

Steps to Reproduce:
1. Go to a second level folder on the Bookmarks Toolbar
2. Select "Open in Tabs"
3. Go back and try to select it again.

Actual Results:  
White spot appears where Open in Tabs should be

Expected Results:  
Open in Tabs is displayed

Default theme used.  Occurs on clean profile, once you add bookmarks to a second
level folder.  I will upload a photo ASAP.
Attached image Picture of white space
Attachment of what i've referred to as "white space."  Again note this does not
include any Live Bookmarks
Version: unspecified → Trunk
Confirmed regression. (Actually, no need to click on "Open In Tabs", just reopen
the menu)
Severity: normal → major
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-aviary1.1?
Keywords: regression
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050412
Firefox/1.0+

I can't reproduce this on windoze.
Flags: blocking-aviary1.1? → blocking-aviary1.1+
-> me for investigation.
Assignee: vladimir+bm → bugs.mano
Priority: -- → P2
Target Milestone: --- → Firefox1.1
The only releavan difference between mac an non-mac is the way we hide the Open
In Tabs menu item (bookmarksmenus.js):

#ifdef XP_MACOSX
  // FIXME: Work around a Mac menu bug with onpopuphidden.  onpopuphiding/hidden
on mac fire
  // before oncommand executes on a menuitem.  For now we're applying a XUL
workaround
  // since it's too late to fix this in the Mac widget code for 1.5.
  hideOpenInTabsMenuItem: function (aTarget)
  {
    setTimeout(function() { BookmarksMenu.realHideOpenInTabsMenuItem(aTarget);
}, 0);
  },
#else
  /////////////////////////////////////////////////////////////////////////////
  // hides the 'Open in Tabs' on popuphidden so that we won't duplicate it -->
  hideOpenInTabsMenuItem: function (aTarget)
  {
    BookmarksMenu.realHideOpenInTabsMenuItem(aTarget);
  },
#endif
Summary: Open in Tabs on second level of Bookmarks Toolbar Folder disappears after selecting → since 20050331: Open in Tabs on second level of Bookmarks Toolbar Folder disappears after reopening the folder (white space)
While the setTimeout hack used here (comment 6) is bogus for XUL (the mac
popuphidden bug only applies to native menus), were there any changes to the way
popuphidden/hiding works in that period (20050330-20050331), from looking at
bonsai, the checkins to content/xul are only bug 233641 and bug 288117.
Asaf, what are the pull dates (including hour) of before/after builds for this
regression?
(In reply to comment #8)
> Asaf, what are the pull dates (including hour) of before/after builds for this
> regression?

The regression area: http://tinyurl.com/8gnr6
Nothing else in the range looks like it could affect this.  I assume a pull by
date for "2005-03-31 07:52" Pacific time shows the bug?  And that a pull by date
for "2005-03-30 07:47" Pacific time does not?
FWIW, when the "Add Bookmark Here" extension is installed and enabled, the "Add
Bookmark Here" entry is "whited out" exactly like the "Open In Tabs" entry.  Not
sure if that might help identify the problem. I originally thought that the
extension might be causing a problem before I retested with a clean profile.

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050531
Firefox/1.0+ 

Josh, can you look at this?
Using latest builds, i also see that in top-level folders (under the bookmarks
toolbar).
Assignee: bugs.mano → nobody
QA Contact: mconnor → bookmarks
I also see this in the pref window (starting from the point Steffen checked in
the context-sensative help fix, _which also changes the hide/shows element_),
when i switch from the General pane to the Advance pane.

Another place is the autocomplete widget (in both content and xul).

In all cases, the area itslef is "clickable", so this has to be a drawing issue.

roc, might it be a regression from Bug 288117?
Assignee: nobody → general
Component: Bookmarks → GFX
Flags: blocking1.8b4?
Flags: blocking1.8b3?
Product: Firefox → Core
QA Contact: bookmarks → ian
Summary: since 20050331: Open in Tabs on second level of Bookmarks Toolbar Folder disappears after reopening the folder (white space) → since 20050331:[Mac] White spcae is shown after redrawing
Target Milestone: Firefox1.1 → mozilla1.8beta3
Summary: since 20050331:[Mac] White spcae is shown after redrawing → since 20050331:[Mac] White space shows up in XUL menu after redrawing
Maybe the checkin for bug 288117 regressed this? It had a bunch of other
repercussions. The other candidate may be 288222, but Asaf has tested that already.
It's entirely possible. If the problem is easily reproducible, someone should be
able to construct a reduced test case...
Flags: blocking1.8b3? → blocking1.8b3-
Whiteboard: needs reduced testcase
This is what I know so far:

When you open a Bookmark Toolbar Folder (in the Bookmark Toolbar) it looks fine.
When you close it and open it again, the options at the bottom are whited out
(by default, the only one existing is 'Open in Tabs' but I also have 'Add
Bookmark Here' - which is whited out as well. The divider line is also whited out.)

I don't think it matters as to if it's top level, second level, etc.

If the problem happens to me on a top level folder, I simply open another folder
and move back, and it is fixed (only for that moment.) This doesn't work for
second and lower levels, though.
This problem now appears in the list that shows up when clicking on the
down-arrow next to the 'back' and 'forward' history buttons. You know, where it
lists the pages that you visited previously? Except, the problem regarding this
area seems to be fairly random for now. I can't find a common reason for this
occurence.

Also, the white space that covers up the text seems to be fairly random as well
(it still covers up normally like the bookmarks, but this time, the # of items
it covers seems to be fairly random. Or maybe I'm not paying enough attention.)
*** Bug 296899 has been marked as a duplicate of this bug. ***
Whiteboard: needs reduced testcase → [no l10n impact] needs reduced testcase
*** Bug 300712 has been marked as a duplicate of this bug. ***
This is the same "invalidate inside of update" issue that we had to fix in cocoa
widgets.
Assignee: general → joshmoz
Component: GFX → Widget: Mac
QA Contact: ian → mac
Actually, it's a little more complex. The window is actually being resized in
the middle of an update, causing some internal dirty region tracking to get
messed up.
So what is actually happening here is that we've called BeginUpdate() on the
window. This causes Carbon to replace the window's visRgn with the intersection
of the visRgn and updateRgn (and stash the visRgn off somewhere). We then go to
resize the window, which calls SizeWindow(), which calculates a new visRgn. Then
we pop out to EndUpdate(), which restores the original visRgn (which is now stale).
Attached patch PatchSplinter Review
This is a not very pretty patch to fix the bug. It does the following:
1. Convert update event handling to be Carbon Event-based. This is so that we
   can handle the BeginUpdate/EndUpdate on our side, rather than having the
   embedder (or nsMacMessagePump) do it. Note that it's redundant to handle
   both kEventWindowUpdate and kEventWindowDrawContent events.

2. Wrap the SizeWindow() call, if in BeginUpdate/EndUpdate pair, in calls that
   save the old update region, break out of the update, resize the window,
   then restore the old update region and go back into BeginUpdate. This fixes
   the bug.
Attachment #189729 - Flags: superreview?(jhpedemonte)
Attachment #189729 - Flags: review?(joshmoz)
(In reply to comment #24)
> Actually, it's a little more complex. The window is actually being resized in
> the middle of an update, causing some internal dirty region tracking to get
> messed up.

Could this cause bug 298502 under some circumstances (very slow cpu for instance) ?
(In reply to comment #27)
> (In reply to comment #24)
> > Actually, it's a little more complex. The window is actually being resized in
> > the middle of an update, causing some internal dirty region tracking to get
> > messed up.
> 
> Could this cause bug 298502 under some circumstances (very slow cpu for
instance) ?

It's possible, but unclear to me how. Also, you're the only person who has seen
bug 298502 as far as I can tell.
Comment on attachment 189729 [details] [diff] [review]
Patch

     // kEventWindowUpdate and kEventWindowDrawContent are only for sheets.
-    PRUint32 typeCount = mIsSheet ? GetEventTypeCount(windEventList) :
-				     GetEventTypeCount(windEventList) - 2;
+    PRUint32 typeCount = GetEventTypeCount(windEventList);

Doesn't look like the comment is necessary any more.  Can be removed.

Otherwise, looks good.
Attachment #189729 - Flags: superreview?(jhpedemonte) → review+
Attachment #189729 - Flags: review?(joshmoz) → review?(mark)
Taking.
Assignee: joshmoz → sfraser_bugs
Comment on attachment 189729 [details] [diff] [review]
Patch

I'm good with this.  r=mento.

One thing: You killed the only thing that calls control_key_down(), which was
debug-only anyway.  Why not kill control_key_down() as well?
Attachment #189729 - Flags: review?(mark) → review+
Attachment #189729 - Flags: approval1.8b4?
Status: NEW → ASSIGNED
Attachment #189729 - Flags: approval1.8b4? → approval1.8b4+
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Flags: blocking1.8b4?
*** Bug 302141 has been marked as a duplicate of this bug. ***
*** Bug 306282 has been marked as a duplicate of this bug. ***
(In reply to comment #20)
> This problem now appears in the list that shows up when clicking on the
> down-arrow next to the 'back' and 'forward' history buttons. 

This bug is putting Firefox into a spinning beach-ball on my system, in the history button - when it happens - and in the bookmark bar - always.
using:
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

Mac OS X v. 10.2.8

500 MHz G3
(In reply to comment #35)
this comment really belongs to bug # 298502
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.