Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 334470

Summary:

Keysequence embedded into updated label of rendered MHandledToolItem

Product:

[Eclipse Project] e4

Reporter:

Brian de Alwis <bsd>

Component:

Assignee:

Project Inbox <e4.ui-inbox>

Status:

RESOLVED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

remy.suen

Version:

unspecified

Target Milestone:

4.1

Hardware:

OS:

Mac OS X - Carbon (unsup.)

Whiteboard:

Attachments:

Description	Flags
Proposed patch to ToolItemRenderer	none
v02	none

Description Brian de Alwis

2011-01-15 20:04:18 EST

The code in ToolItemRenderer#setItemText() causes the bound keysequence to be embedded in the corresponding ToolItem's label text.  This doesn't seem to be the desired action, does it?

Comment 1 Remy Suen

2011-01-16 06:10:29 EST

Is this a new bug (presumably caused by the renderer changes on Friday to support translations)?

Comment 2 Oleg Besedin

2011-01-17 09:45:37 EST

(In reply to comment #1)
> Is this a new bug (presumably caused by the renderer changes on Friday to
> support translations)?

I don't think so. The code is:

private void setItemText(MToolItem model, ToolItem item) {
   String text = model.getLabel();
   if (model instanceof MHandledItem) {
      ...
      TriggerSequence sequence = [BindingService].getBestSequenceFor(handledItem);
      if (sequence != null) {
         text = text + '\t' + sequence.format();
      }
   item.setText(text);
   }
   ...
}

The changes for translation replaced
   String text = model.getLabel();
with
   String text = model.getLocalizedLabel();

Comment 3 Brian de Alwis

2011-02-16 18:29:20 EST

Created attachment 189144 [details]
Proposed patch to ToolItemRenderer

I suppose this problem isn't commonly seen as most ToolItems in actual use specify an icon.  I think the code came from a copy-n-paste from HandledMenuItemRenderer.

Embedding the keysequence is definitely not correct for the ToolItem label.  But it would make sense for the tooltip?

Comment 4 Brian de Alwis

2011-03-29 09:07:18 EDT

Created attachment 192083 [details]
v02

setItemText() doesn't need to compute the tooltip at all; that's done in getToolTipText() directly below.

Comment 5 Brian de Alwis

2011-03-29 09:10:42 EDT

Patch v02 committed to HEAD