A11y: Replace non-semantic div buttons with <button> in URLPopUpSection #13724

omkarhole · 2025-12-03T17:40:04Z

Replaced non-semantic

elements with proper elements to improve keyboard accessibility, screen reader support, and WCAG compliance. Preserved styling, IDs, aria-labels, and existing behavior.

Changes

Replaced L.DomUtil.createWithId('div', …) with 'button'
Removed ARIA roles no longer needed

Preserved:

IDs
classes
aria-labels
onclick handlers
icon images
No visual or behavioral regression

Issue Link:#12017

@rparth07 @Darshan-upadhyay1110

Copilot

Pull request overview

This PR improves accessibility by replacing non-semantic <div role="button"> elements with proper <button> elements in the URL popup section, aligning with WCAG guidelines and improving keyboard navigation and screen reader support.

Key Changes:

Replaced three <div role="button"> elements with native <button> elements (copy, edit, and remove buttons)
Removed redundant role="button" attributes since native buttons have implicit button semantics
Fixed a bug where the edit button incorrectly used copyLinkText for its aria-label instead of editLinkText

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

omkarhole · 2025-12-03T17:54:13Z

Hi,
This PR replaces the non-semantic

elements inside URLPopUpSection.ts with native elements to align with A11y best practices. The original behavior, styling, aria-labels, icons, and event handlers remain unchanged.

This improves:

Keyboard accessibility (automatic Enter/Space behavior)
Screen reader semantics
WCAG compliance
Reduces need for ARIA patches
Let me know if any additional updates or tests are needed. Thanks!

Darshan-upadhyay1110 · 2025-12-07T12:14:43Z

Thanks @omkarhole! I noticed the Change-Id is missing. Please make sure your commits include both a Signed-off-by (DCO) line and a Change-Id.

Add Change-Id

To automatically generate a Change-Id:

Copy the commit hook:

cp .git-hooks/commit-msg .git/hooks/commit-msg

Amend the commit so the Change-Id is added:
```
git commit --amend
git push -f
```

omkarhole · 2025-12-07T12:25:30Z

Thanks for the feedback. I’ve added the Change-Id and updated the commit as requested.
The Signed-off-by line is also included @Darshan-upadhyay1110

rparth07

Thanks @omkarhole for patch:)
Overall changes are good! This enhancement will make it better :)

rparth07 · 2025-12-08T04:55:08Z

browser/src/canvas/sections/URLPopUpSection.ts

 		app.LOUtil.setImage(imgCopyBtn, 'lc_copyhyperlinklocation.svg', app.map);
 		imgCopyBtn.setAttribute('width', 18);
 		imgCopyBtn.setAttribute('height', 18);
 		imgCopyBtn.setAttribute('alt', copyLinkText);


As we have aria-label on parent button. we can avoid populating alt attribute on <img> to avoid confusion for assistive technology. we can set it to empty. i.e. imgCopyBtn.setAttribute('alt', '');

rparth07 · 2025-12-08T04:55:35Z

browser/src/canvas/sections/URLPopUpSection.ts

 		app.LOUtil.setImage(imgEditBtn, 'lc_edithyperlink.svg', app.map);
 		imgEditBtn.setAttribute('width', 18);
 		imgEditBtn.setAttribute('height', 18);
 		imgEditBtn.setAttribute('alt', editLinkText);


same as above, we can update alt to empty string.

rparth07 · 2025-12-08T04:56:44Z

browser/src/canvas/sections/URLPopUpSection.ts

 		app.LOUtil.setImage(imgRemoveBtn, 'lc_removehyperlink.svg', app.map);
 		imgRemoveBtn.setAttribute('width', 18);
 		imgRemoveBtn.setAttribute('height', 18);
 		imgRemoveBtn.setAttribute('alt', removeLinkText);


same as above, we can update alt to empty string.

rparth07 · 2025-12-08T05:05:53Z

URL Pop-up dialog:
Before:

After:

I see this will affect UI as above.
cc: @Darshan-upadhyay1110 @pedropintosilva

Darshan-upadhyay1110

@omkarhole the URL popup UI fully broken now it's not looking okay. A11y should not change the UI look the buttons should be consistent as it was before....

omkarhole · 2025-12-08T05:35:57Z

sure i will fix it .

omkarhole · 2025-12-13T20:06:24Z

attempted to verify the fix using the Ona development environment, but environment creation requires credit-card verification.
The change is limited to CSS and semantic HTML, and has been manually verified to preserve the previous UI while improving accessibility. @Darshan-upadhyay1110

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

browser/src/canvas/sections/URLPopUpSection.ts

browser/css/cool.css

rparth07 · 2025-12-22T06:27:15Z

@omkarhole Merge commits are not allowed... can you please rebase?

omkarhole · 2025-12-23T13:46:55Z

Hi @rparth07 , @Darshan-upadhyay1110
I’ve rebased the branch on top of upstream master and removed the merge commit.
Please let me know if anything else is needed. Thanks!

eszkadev · 2025-12-31T08:01:08Z

Hi @rparth07 , @Darshan-upadhyay1110 I’ve rebased the branch on top of upstream master and removed the merge commit. Please let me know if anything else is needed. Thanks!

hi @omkarhole I see it still has merge commit on top: 47da10f

could you please rebase it instead?
how to do it:

git log <-- see commits in your branch
git reset HEAD~1 --hard <-- remove 1 patch from the top
git branch -u origin/main <-- follow branch main
git pull -r <-- pull form origin/main with rebasing of your branch on top
git log <-- see there is origin/main commit and on top your changes
push...

rparth07 · 2026-01-01T14:30:16Z

browser/css/cool.css

 	vertical-align: middle;
 }

 .hyperlink-popup-btn {


I can still see that the button UI is affected. This CSS is being overridden. Could you please check again?

Hii , @rparth07 . I Have Check the changes on my local machine and it dosent shows button UI affected

would you please recheck it again ?

hi @omkarhole, can you please do a make clean and built it again and check.
I can see the UI affected.

till commit c191d64771 UI is normal
i.e.

Screencast.from.2026-01-02.11-58-25.webm

on commit 3fa947dda2 UI is broken
i.e.

Screencast.from.2026-01-02.12-01-14.webm

@rparth07 which browser are you using chrome or other ?

Chromium Version 143.0.7499.146

@rparth07
i have check the again CSS but it dosent show any UI Broken and i use google chrome browser in that i works fine ! .would you please check on different browser on you local system please .

Darshan-upadhyay1110 · 2026-01-02T13:13:36Z

hi @omkarhole I also tested this patch

here how it looks , I can also reproduce the issue what @rparth07 mentioned in above comments...

Screencast.from.2026-01-02.17-10-26.webm

Using chrome : Version 143.0.7499.109 (Official Build) (64-bit)

Same goes to Fire fox :

so i would suggest you do a clean build ? and then test ?

omkarhole · 2026-01-03T05:53:29Z

@Darshan-upadhyay1110 , @rparth07 i have done clean code and make some changes and also created video and now the UI looks Good . please go through video and check on your local machine also

video:

Untitled.video.-.Made.with.Clipchamp.1.mp4

Darshan-upadhyay1110 · 2026-01-05T05:25:37Z

Please add change ID and DCO. And i see the cypress is not happy can you please check if it's passes the cases in local or not ?

rparth07 · 2026-01-05T13:26:53Z

hi @omkarhole, it looks like you accidentally includes already merged commits in this PR. can you please fix it. Thanks :)

- Replaced div role='button' elements with proper button elements - Removed redundant ARIA roles (button role is implicit in button elements) - Fixed aria-label on edit button (was using copyLinkText instead of editLinkText) - Set alt='' on images since buttons already have aria-labels (avoiding redundancy) - Added CSS reset for button elements to maintain consistent styling - Improved keyboard accessibility and screen reader support Issue: CollaboraOnline#12017 Signed-off-by: omkar hole <[email protected]> Change-Id: I06f6da85ffbaa656962fad3207c6a4c132d07572

omkarhole · 2026-01-05T14:42:54Z

@rparth07 i have fix the branch and make it clean you can review it .

Copilot AI review requested due to automatic review settings December 3, 2025 17:40

github-project-automation bot added this to Collabora Online Dec 3, 2025

github-project-automation bot moved this to To Review in Collabora Online Dec 3, 2025

Copilot started reviewing on behalf of omkarhole December 3, 2025 17:40 View session

Copilot finished reviewing on behalf of omkarhole December 3, 2025 17:41

omkarhole changed the title ~~A11y: Replace <div role="button"> with <button> in URLPopUpSection~~ A11y: Replace non-semantic div buttons with <button> in URLPopUpSection Dec 3, 2025

Copilot AI reviewed Dec 3, 2025

View reviewed changes

omkarhole force-pushed the replace-role-div-with-button- branch from b31a89d to 0bf8743 Compare December 3, 2025 17:49

Darshan-upadhyay1110 requested review from Darshan-upadhyay1110 and rparth07 December 7, 2025 07:14

omkarhole force-pushed the replace-role-div-with-button- branch from 0bf8743 to d9c26f7 Compare December 7, 2025 12:23

rparth07 reviewed Dec 8, 2025

View reviewed changes

Darshan-upadhyay1110 requested changes Dec 8, 2025

View reviewed changes

github-project-automation bot moved this from To Review to In Progress in Collabora Online Dec 8, 2025

omkarhole requested review from Darshan-upadhyay1110, Copilot and rparth07 December 13, 2025 20:09

Copilot started reviewing on behalf of omkarhole December 19, 2025 09:18 View session

Copilot AI reviewed Dec 19, 2025

View reviewed changes

omkarhole force-pushed the replace-role-div-with-button- branch from 9fb23eb to 2b804be Compare December 23, 2025 13:44

omkarhole force-pushed the replace-role-div-with-button- branch from 47da10f to 3fa947d Compare January 1, 2026 07:32

rparth07 requested changes Jan 1, 2026

View reviewed changes

omkarhole requested a review from rparth07 January 2, 2026 12:14

omkarhole force-pushed the replace-role-div-with-button- branch 2 times, most recently from 6cb86fe to 5fc6582 Compare January 5, 2026 07:08

omkarhole force-pushed the replace-role-div-with-button- branch from 5fc6582 to 642cf20 Compare January 5, 2026 14:40

A11y: Replace non-semantic div buttons with <button> in URLPopUpSection #13724

Are you sure you want to change the base?

A11y: Replace non-semantic div buttons with <button> in URLPopUpSection #13724

Conversation

omkarhole commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

omkarhole commented Dec 3, 2025

Uh oh!

Darshan-upadhyay1110 commented Dec 7, 2025

Add Change-Id

Uh oh!

omkarhole commented Dec 7, 2025

Uh oh!

rparth07 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rparth07 commented Dec 8, 2025

Uh oh!

Darshan-upadhyay1110 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

omkarhole commented Dec 8, 2025

Uh oh!

omkarhole commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rparth07 commented Dec 22, 2025

Uh oh!

omkarhole commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eszkadev commented Dec 31, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Darshan-upadhyay1110 commented Jan 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

omkarhole commented Jan 3, 2026

Uh oh!

Darshan-upadhyay1110 commented Jan 5, 2026

Uh oh!

rparth07 commented Jan 5, 2026

Uh oh!

omkarhole commented Jan 5, 2026

Uh oh!

Reviewers

Assignees

Labels

omkarhole commented Dec 3, 2025 •

edited

Loading

Darshan-upadhyay1110 left a comment •

edited

Loading

omkarhole commented Dec 13, 2025 •

edited

Loading

omkarhole commented Dec 23, 2025 •

edited

Loading

Darshan-upadhyay1110 commented Jan 2, 2026 •

edited

Loading