Closed Bug 481767 Opened 12 years ago Closed 11 years ago

Tag field overlay issue when 2 or more bookmarks in the same tag

Categories

(Firefox :: Bookmarks & History, defect)

3.5 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 3.6a1

People

(Reporter: aakashd, Assigned: mak)

References

Details

(Keywords: verified1.9.1)

Attachments

(3 files)

An overlay of the tags field occurs when 2 or more bookmarks are added to the same tag folder. Screenshots of the issue when the library is at the default size and at an extended size are attached to this bug.


1. Go to the Bookmarks Library
2. Select an un-tagged bookmark and add a tag to it. 
3. In that tag folder, view the details pane of the tagged bookmark.
4. Select another un-tagged bookmark and add the same tag from step 2.
5. In the tag folder, view the details pane (specifically the tags field) of both of the tagged bookmarks.
Aakash, please add the build id to a bug so we know, which build shows this behavior. Further check Shift+Cmd+4 on you machine to make smaller screenshot which makes the problem more focusable. Thanks.

Same happens on Windows with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3) Gecko/20090304 Firefox/3.1b3. It looks like that another white field is overlaying the textbox. Probably a leftover from the autocomplete? If you resize the dialog the overlay moves to the right and shows at some point the whole tag.
OS: Mac OS X → All
Hardware: x86 → All
i had seen this time ago, but i resolved it as WFM since i was unable to reproduce, was bug 469305.
With the steps here it is reproducible each time.
cc-ing dao, i suspect is somehow related to autocomplete and emptytext
Duplicate of this bug: 469305
Does this still fail if the tags field doesn't have emptytext?
(In reply to comment #2)
> Aakash, please add the build id to a bug so we know, which build shows this
> behavior. Further check Shift+Cmd+4 on you machine to make smaller screenshot
> which makes the problem more focusable. Thanks.
> 
> Same happens on Windows with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
> rv:1.9.1b3) Gecko/20090304 Firefox/3.1b3. It looks like that another white
> field is overlaying the textbox. Probably a leftover from the autocomplete? If
> you resize the dialog the overlay moves to the right and shows at some point
> the whole tag.

Ack, crap, here's the build ID. Sorry about that guys.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3) Gecko/20090305 Firefox/3.1b3 ID:20090305133223
(In reply to comment #7)
> Does this still fail if the tags field doesn't have emptytext?

Any simple way to test this?
(In reply to comment #9)
> (In reply to comment #7)
> > Does this still fail if the tags field doesn't have emptytext?

If it happens once it will happen for everything, like clicking through bookmarks under the tags container and also under the bookmarks root folders. Someone has to reopen the Library to get rid off.
Any chance to get it fixed for 3.5? I run into this problem a couple of times while running the Library BFT.
Flags: blocking-firefox3.5?
(In reply to comment #11)
> Any chance to get it fixed for 3.5? I run into this problem a couple of times
> while running the Library BFT.
Does this also happen in Firefox 3.0?  If so, this probably shouldn't block.  That doesn't mean we wouldn't take a patch...
A quick check with the same steps in my BFT profile doesn't show this issue by running Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.8) Gecko/2009032609 Firefox/3.0.8 (.NET CLR 3.5.30729). So it should be a regression from 3.0.
Reproducible, and really makes the tags field of the library in tag container items look broken, so marking blocking.
Flags: blocking-firefox3.5? → blocking-firefox3.5+
(In reply to comment #7)
> Does this still fail if the tags field doesn't have emptytext?

it still fails if i remove emptytext from the field.
while if i remove the autocomplete everything works fine, so it's related to the autocomplete widget.
looking with dom inspector, the popupset ends up on top of the input field.
Attached patch patch v1.0Splinter Review
there is code for a similar situation in mailnews core bug 117952
and in xul.css

787 /* The following rule is here to fix bug 96899 (and now 117952).  
788    Somehow trees create a situation
789    in which a popupset flows itself as if its popup child is directly within it
790    instead of the placeholder child that should actually be inside the popupset.
791    This is a stopgap measure, and it does not address the real bug.  */
792 .autocomplete-result-popupset {
793   max-width: 0px;
794   width: 0 !important;
795 }

Looks like this has been workarounded, and the base reason is not known, nor i can find a bug filed to address the real bug.

This applies the same workaround that is in xpfe autocomplete.css to xul.css, and is working. I would suggest taking this and filing a bug to find the real issue.
Comment on attachment 376017 [details] [diff] [review]
patch v1.0

Maybe Neil knows something about this.
Attachment #376017 - Flags: review?(enndeakin)
taking for now.
Assignee: nobody → mak77
Whiteboard: [has patch][needs review enn]
What exactly is the bug here? What am I looking for in the screenshots?
(In reply to comment #21)
> What exactly is the bug here? What am I looking for in the screenshots?

In the tags field in the screenshot, the tag is truncated - you can only see part of the first letter.

However, I can't reproduce this now. I reproduced it easily when I marked it blocking...
asking qawanted. i really can't reproduce it anymore. maybe we need an unregression-window? :)
Keywords: qawanted
i can still reproduce every time with the above steps
Neil, practically the autocomplete popupset ends up inside the autocomplete field, and covers most of it in some situation, so as dietrich said you only see half of the first letter.
The same bug was observed many times before (as you can see in bug i reported in comment 18) and nobody found the real cause (i think it is also because there isn't an open bug filed upon that?). My patch is syncing workaround code between xpfe and xul.css, and from my tests works like a charm (so i suggest syncing that code and filing a bug in autocomplete to find the underlying cause).
removing all regwindow requests, since this is a widget bug, we have this from the start, when we added autocomplete to tags field.
(In reply to comment #26)
> My patch is syncing workaround code between xpfe and xul.css,
> and from my tests works like a charm

What is the workaround for? If this was added to xpfe, maybe the other Neil is a better reviewer here.
Comment on attachment 376017 [details] [diff] [review]
patch v1.0

I think the point is allowing the popupset box to shrink, so that it does not push on the field input contents.

I've found that xpfe addition of min-width and min-height has been done in bug 268780, that is still open, that workarounded the issue for the locationbar, and is r+sr=roc as a temporary fix.

Asking r+sr to roc, and marking this dependant on that.
Attachment #376017 - Flags: superreview?(roc)
Attachment #376017 - Flags: review?(roc)
Attachment #376017 - Flags: review?(enndeakin)
Depends on: 268780
Whiteboard: [has patch][needs review enn] → [has patch][needs review roc]
Also Boris looked into that bug, so cc-ing him.
Why am I the right reviewer for this? I'm not a toolkit peer.
roc, you reviewed the patch for xpfe, i know you're not a toolkit peer, but you can probably give us some detail that will allow a toolkit peer (Neil?) to review this. Btw i don't think will make a big difference if you're not a toolkit peer in this case, since sounds like the bug is at a lower level than the toolkit widget.
I have occasionally got in trouble for not asking for toolkit peer review when I should, so I would prefer you to get a toolkit peer. There's plenty of them.
Comment on attachment 376017 [details] [diff] [review]
patch v1.0

I understand your point, opinions are not reviews though, and are still highly appreciated.
Attachment #376017 - Flags: superreview?(roc)
Attachment #376017 - Flags: superreview?(neil)
Attachment #376017 - Flags: review?(roc)
Attachment #376017 - Flags: review?(neil)
Whiteboard: [has patch][needs review roc] → [has patch][needs review neil]
Attachment #376017 - Flags: superreview?(neil)
Attachment #376017 - Flags: superreview+
Attachment #376017 - Flags: review?(neil)
Attachment #376017 - Flags: review+
Blocks: 492444
Whiteboard: [has patch][needs review neil] → [has patch][has review]
http://hg.mozilla.org/mozilla-central/rev/48c0f9ab08ca
Whiteboard: [has patch][has review]
Target Milestone: --- → Firefox 3.6a1
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Looks great. I'm not able to reproduce it anymore on trunk and 1.9.1.

Verified fixed with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090513 Minefield/3.6a1pre ID:20090513034237

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b5pre) Gecko/20090513 Shiretoko/3.5b5pre ID:20090513034609
Status: RESOLVED → VERIFIED
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.