Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#3637 closed bug (fixed)

[preferences/FileTypes] menuField text is clipped(easy)

Reported by: chico Owned by: stippi
Priority: normal Milestone: R1
Component: Preferences/FileTypes Version: R1/Development
Keywords: gsoc2010 Cc: pprahul@…
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

  1. open beos/preferences/FileTypes
  2. choose application->bookmarks in left List box

Result: you will see menufield text "application/x-vnd.Be-NPOS" clipped in the preferred Application. Expected: the full text "application/x-vnd.Be-NPOS" will be showed in preferred Application Suggestion: we can limite the text, for example, there are only 10 characters to describe an application; or we resize the window to show the whole text

Attachments (3)

FileType-preferredApplication-bookmark.png (188.9 KB ) - added by chico 10 years ago.
filetypes.patch (3.0 KB ) - added by yourpalal 10 years ago.
patch to enable truncation of BMenuItem's, specifically to fix this bug
filetypes.2.patch (2.9 KB ) - added by yourpalal 10 years ago.
patch to enable truncation of BMenuItem's, specifically to fix this bug

Download all attachments as: .zip

Change History (26)

comment:1 by pprahul, 10 years ago

Cc: pprahul@… added

I would like to work on this. I need some suggestions on what can be done. These are the various options available. Please suggest any other options too.

  1. Should the text be limited ? If so, what is the criteria to clip it?
  2. Should the width of the menufield be increased? (I don't think thats a good idea, as it would effect the nice GUI of the application)
  3. Or else, we could re-arrange the buttons and menufield, so that, it fits into that "Preferred Application" area ?

comment:2 by axeld, 10 years ago

Version: R1/pre-alpha1R1/Development

I think the BMenuField must not get larger than it should, and the text should be clipped within the menu field accordingly (this should happen automatically, though, I'm not sure this works yet).

in reply to:  2 comment:3 by pprahul, 10 years ago

Replying to axeld:

I think the BMenuField must not get larger than it should, and the text should be clipped within the menu field accordingly (this should happen automatically, though, I'm not sure this works yet).

Thanks for the input.
So, does it imply that the text in the BMenuField should be clipped in the following way :
original text : application/x-vnd.Haiku-urlwrapper
new text 1 : application/x-vnd.H... (ignoring the last part)
new text 2 : .../x-vnd.Haiku-urlwrapper (ignoring the first part)

(I think the second one makes more sense, though it is only a personal opinion)
And also, is there any method that can be called on this MenuField that will do the clipping automatically ?

comment:4 by humdinger, 10 years ago

Will this be a general feature for BMenuField or only FileTypes specific?

There are other, maybe more obvious cases of truncated and overwriting pop-up menus, like the account menu when resizing a new mail window (which is of course a Mail bug).
Normally, every preferred app should have a proper entry in that menu (like "urlwrapper" for bookmarks). The signature shouldn't appear there at all. The only instance at this moment is "Clockwerk Object", but that's an optional package in need of much further development.

WRT trunkating style, it should be in the middle. See long document windows in the Deskbar.

comment:5 by axeld, 10 years ago

The truncating should always happen automatically in BMenuField - if the field is too small to show the whole text, it must be truncated.

The signatures should only appear in the preferred app menu when the application in question has been removed. However, it will also show if that helps to differentiate two entries with the same name.

comment:6 by pprahul, 10 years ago

So, I am a bit confused on what to be done with this bug.

From what I have seen from the code, these are my conclusions. Please point out where I've gone wrong, so that we can improve the code.

  1. The BMenuField should automatically truncate the text (which is a common property, and nothing related to this bug). But it doesn't do the same function (truncate) here (WRT PreferredAppMenu).

If that is the case, then : I can't find any explicit call to TruncateString() , neither from BMenuField, nor from PreferredAppMenu.cpp.
Could this be the cause of this problem ?

  1. Or else, the BMenuField is working properly (and truncates too), but WRT PreferredAppMenu it is not truncated properly.

If that is the case, then we will need to make a specific change, only for PreferredAppMenu, which I don't think is the best way out.

What is your take on this ?

comment:7 by stippi, 10 years ago

If you couldn't find the TruncateString() method being used in drawing code, then that's most likely the issue. You have to know that BMenuFields internally use a BMenuBar, which is a subclass of BMenu. The rendering of the label is done using BMenuItem drawing code. Usually, a menu uses as much space as it needs on the screen, so that's probably why the string truncation is missing. In BMenuItem, there should be code similar to DrawString(Label(), ...);. This needs to change to:

BString truncatedLabel(Label());
font.TruncateString(&truncatedLabel, ...);
DrawString(truncatedLabel.String(), ...);

... with the availabel width passed to font.TruncateString(). If there isn't already the font variable around, then you can get it via Menu()->GetFont(&font). But to do this in every BMenuItem (if it isn't already done) is a small performance problem.

comment:8 by jackburton, 10 years ago

BMenuItem::DrawContent uses TruncateString already, but probably it's never triggered, since the width of the menuitem is already set to the item full string width.

        // truncate if needed
	// TODO: Actually, this is still never triggered
	if (fBounds.Width() > labelWidth)
		fSuper->DrawString(fLabel);
	else {
		char *truncatedLabel = new char[strlen(fLabel) + 4];
		TruncateLabel(fBounds.Width(), truncatedLabel);
		fSuper->DrawString(truncatedLabel);
		delete[] truncatedLabel;
	}

comment:9 by pprahul, 10 years ago

I was trying to find the reason behind it not being not getting triggered.

I traced the method calls to find out the reason. I also explicity called the TruncateLabel() so as to see if the function is working properly. But to my surprise, the Truncation never worked in this case.

So, can anyone tell me a case where this truncateLabel() is working without any problem. Any GUI example where it is being used (and is working properly). This would allow me to understand how it is working in such cases, and may possibly lead to a solution for this one too.

comment:10 by stippi, 10 years ago

TruncateString() may have a bug where the ellipsis character is wider than the char it chopped off, but in general, it is working. That it doesn't work in the above case is most likely the cause of a wrong width being passed to the BMenuItem. It probably thinks it is wider than it actually is. (The super item that is part of the BMenuBar.)

comment:11 by pprahul, 10 years ago

Thanks for the input....
Can you give me an instance (A GUI) where it is working correctly...so that I could debug with reference to it...

comment:12 by axeld, 10 years ago

A problem in the case of FileTypes is that is uses a BMenuField with a variable width, and that one is usually as large as needed. It puts the field into its own view to have it cut off at the right width.

The BMenuField should make sure that it does not grow beyond the bounds of its parent. And I guess that part is currently missing.

comment:13 by yourpalal, 10 years ago

Hi, is development ongoing on this bug? If not I would like to take a crack at it.

comment:14 by stippi, 10 years ago

Please have a go at it! Thanks for looking into it!

in reply to:  13 comment:15 by pprahul, 10 years ago

Replying to yourpalal:

Hi, is development ongoing on this bug? If not I would like to take a crack at it.

Hi, I worked on it for few days, but then didn't follow it up later. So, go ahead please.

comment:16 by yourpalal, 10 years ago

Okay, I think I've got it! I tested other apps to see if this patch caused any regressions, and I didn't find any, but my testing wasn't extensive. Either way, I don't think it should cause any problems.

comment:17 by stippi, 10 years ago

Thanks, this looks like the correct fix indeed. However, a few nitpicks on the patch :-) - don't put spaces after and before enclosing parenthesis, and I *think* the hardcoded -15 could be avoided by looking at the "item margins". In BMCPrivate.cpp, in _BMCMenuBar_::_Init(), you can see that it uses SetItemMargins() and indeed BMenuField has the popup indicator optional, so hardcoding this isn't so good. Hopefully you can get to the item margins in that BMenuItem method, but I think it should be the case. Thanks a lot for having looked into this!

by yourpalal, 10 years ago

Attachment: filetypes.patch added

patch to enable truncation of BMenuItem's, specifically to fix this bug

comment:18 by yourpalal, 10 years ago

Ignore the patch I just posted, I hadn't seen your comment stippi.

comment:19 by yourpalal, 10 years ago

Okay, I woke up this morning thinking basically the same thing, re-wrote that section using _BCMMenuBar_::GetPopupIndicatorEnabled() (or something like that), posted that patch, read your post... haha Anyway, I'v re-written it with GetItemMargins() which is definitely the best way to do it! I took out those spaces and I think everything looks good. So I'll upload the new patch now. Thanks for your stippi!

by yourpalal, 10 years ago

Attachment: filetypes.2.patch added

patch to enable truncation of BMenuItem's, specifically to fix this bug

comment:20 by stippi, 10 years ago

Owner: changed from axeld to stippi
Status: newin-progress

Thanks a bunch!

comment:21 by stippi, 10 years ago

Resolution: fixed
Status: in-progressclosed

Applied in hrev35739 and hrev35740. Thanks a lot and sorry for mixing it up somewhat with changes of my own.

comment:22 by yourpalal, 10 years ago

Haha, I'm happy to help! Thanks for your pointers/hints!

comment:23 by mmadia, 9 years ago

Keywords: gsoc2010 added
Note: See TracTickets for help on using tickets.