Skip to content

Comments

Adapt HistoryTextWrapper#notifyListeners#3703

Merged
HeikoKlare merged 1 commit intoeclipse-platform:masterfrom
vi-eclipse:amartya4256/historyTextWrapper_Dpi_change
Jan 27, 2026
Merged

Adapt HistoryTextWrapper#notifyListeners#3703
HeikoKlare merged 1 commit intoeclipse-platform:masterfrom
vi-eclipse:amartya4256/historyTextWrapper_Dpi_change

Conversation

@amartya4256
Copy link
Contributor

@amartya4256 amartya4256 commented Jan 26, 2026

This PR adapts the notifyListeners method in HistoryTextWrapper to call super#notifyListeners since not calling it changes the underlying default behaviour of its superclasses leading to faulty behavior, e.g. it breaks the chain of DPIChange processing leading to a broken drop down button in the HistoryTextWraper on DPIChange Event.

Bug Explained

When a file editor has the Ctrl+F search box open and the editor window is moved across screens with different DPI/scaling settings, the search box does not scale properly.

Steps to Reproduce:

  1. Open a file in the editor.
  2. Press Ctrl+F to open the search box.
  3. Move the editor window from one monitor to another with a different zoom/DPI setting.
  4. Observe that the search box remains the same size and does not adjust to the new DPI.
Image

Reason

The class HistoryTextWrapper overrides the method notifyListeners:

@Override
public void notifyListeners(int eventType, Event event) {
	textBar.notifyListeners(eventType, event);
}

This method doesn't call super.notifyListeners(eventType, event), hence, the chain of DPIChange processing is broken, leading to the toolbar under the HistoryTextWrapper never scaling.

@amartya4256 amartya4256 force-pushed the amartya4256/historyTextWrapper_Dpi_change branch from 49c5202 to 2b74f4f Compare January 26, 2026 15:49
@github-actions
Copy link
Contributor

github-actions bot commented Jan 26, 2026

Test Results

 3 018 files  ±0   3 018 suites  ±0   2h 16m 8s ⏱️ + 7m 23s
 8 229 tests ±0   7 981 ✅ ±0  248 💤 ±0  0 ❌ ±0 
23 511 runs  ±0  22 720 ✅ ±0  791 💤 ±0  0 ❌ ±0 

Results for commit 5e42659. ± Comparison against base commit 911ae9f.

♻️ This comment has been updated with latest results.

@amartya4256 amartya4256 force-pushed the amartya4256/historyTextWrapper_Dpi_change branch from 2b74f4f to 091ec64 Compare January 27, 2026 08:56
This commit adapts the notifyListeners method in HistoryTextWrapper to
call super#notifyListeners since not calling it changes the underlying
default behaviour of its superclasses leading to faulty behavior, e.g.
it breaks the chain of DPIChange processing leading to a broken drop down
button in the HistoryTextWraper on DPIChange Event.
@amartya4256 amartya4256 force-pushed the amartya4256/historyTextWrapper_Dpi_change branch from 091ec64 to 5e42659 Compare January 27, 2026 11:22
@amartya4256 amartya4256 changed the title Remove HistoryTextWrapper#notifyListeners Adapt HistoryTextWrapper#notifyListeners Jan 27, 2026
Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The missing super call obviously breaks proper notification handling of the composite. Every other control overwriting that method does as super call as well. It seems to have been an unnoticed oversight in the original implementation of this class in #1990. At least there is no mention why exactly this kind of implementation is necessary.

@HeikoKlare HeikoKlare merged commit a5bdb07 into eclipse-platform:master Jan 27, 2026
18 checks passed
@HeikoKlare HeikoKlare deleted the amartya4256/historyTextWrapper_Dpi_change branch January 27, 2026 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ctrl+f search box is not relayouted on DPI change

2 participants