Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reimplement the progress display in wide-layout attachments #12424

Merged

Conversation

squelart
Copy link
Contributor

@squelart squelart commented Apr 5, 2023

31c3e79

Reimplement the progress display in wide-layout attachments
https://bugs.webkit.org/show_bug.cgi?id=255100
rdar://problem/107716507

Reviewed by Aditya Keerthi.

When a wide-layout attachment has an attribute "progress" with a number, it is now directly handled by the top-level attachment element, by displaying a pie chart; previously it was handled by the inner attachment's legacy code.

* Source/WebCore/html/HTMLAttachmentElement.cpp:
(WebCore::attachmentProgressIdentifier):
(WebCore::HTMLAttachmentElement::ensureModernShadowTree):
(WebCore::HTMLAttachmentElement::updateProgress):
(WebCore::HTMLAttachmentElement::parseAttribute):
* Source/WebCore/html/HTMLAttachmentElement.h:
* Source/WebCore/html/shadow/attachmentElementShadow.css:
(div#attachment-progress):

Canonical link: https://commits.webkit.org/262758@main

558cd0c

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ›  gtk
❌ πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ›  tv βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ api-gtk
βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@squelart squelart self-assigned this Apr 5, 2023
@squelart squelart force-pushed the eng/attachment-progress-pie branch from ed6b445 to c86f6a4 Compare April 6, 2023 18:29
@squelart squelart added the HTML Editing For bugs in HTML editing support (including designMode and contentEditable). label Apr 6, 2023
@squelart squelart force-pushed the eng/attachment-progress-pie branch from c86f6a4 to 2d4e5f1 Compare April 6, 2023 20:16
Source/WebCore/html/HTMLAttachmentElement.cpp Outdated Show resolved Hide resolved
Source/WebCore/html/HTMLAttachmentElement.cpp Outdated Show resolved Hide resolved
Comment on lines 93 to 94
/* FIXME: Combine border into attachment-progress above, when rdar://107621207 is fixed. */
div#attachment-progress-circle {
Copy link
Member

@pxlcoder pxlcoder Apr 6, 2023

Choose a reason for hiding this comment

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

Would we also be able to remove this div entirely when rdar://107621207 is fixed? Other than that, it's not obvious to me why we need both.

Copy link
Contributor Author

@squelart squelart Apr 6, 2023

Choose a reason for hiding this comment

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

Oh, that's what I meant: Move the border styling to the above div, and remove this div. I'll rephrase for clarity...

@squelart squelart force-pushed the eng/attachment-progress-pie branch from 2d4e5f1 to b51a528 Compare April 6, 2023 21:20
@squelart squelart force-pushed the eng/attachment-progress-pie branch from b51a528 to 4ad821d Compare April 6, 2023 21:30
@@ -54,6 +54,7 @@
#include <wtf/IsoMallocInlines.h>
#include <wtf/UUID.h>
#include <wtf/URLParser.h>
#include <wtf/text/StringBuilder.h>
Copy link
Member

Choose a reason for hiding this comment

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

This include can be removed now.

@pxlcoder
Copy link
Member

pxlcoder commented Apr 6, 2023

Please move this out of draft status prior to merging (I wonder if it will even let you merge?).

@squelart squelart force-pushed the eng/attachment-progress-pie branch from 4ad821d to 558cd0c Compare April 6, 2023 21:44
@squelart squelart marked this pull request as ready for review April 6, 2023 22:09
@squelart
Copy link
Contributor Author

squelart commented Apr 6, 2023

Please move this out of draft status prior to merging (I wonder if it will even let you merge?).

Strange, I don't remember making it a draft!? Anyway, back to normal now. I'll let the tests finish before merging...

@cdumez
Copy link
Contributor

cdumez commented Apr 6, 2023

Please move this out of draft status prior to merging (I wonder if it will even let you merge?).

It wouldn't let you merge in my experience.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 7, 2023
@squelart squelart added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged merge-queue Applied to send a pull request to merge-queue labels Apr 7, 2023
https://bugs.webkit.org/show_bug.cgi?id=255100
rdar://problem/107716507

Reviewed by Aditya Keerthi.

When a wide-layout attachment has an attribute "progress" with a number, it is now directly handled by the top-level attachment element, by displaying a pie chart; previously it was handled by the inner attachment's legacy code.

* Source/WebCore/html/HTMLAttachmentElement.cpp:
(WebCore::attachmentProgressIdentifier):
(WebCore::HTMLAttachmentElement::ensureModernShadowTree):
(WebCore::HTMLAttachmentElement::updateProgress):
(WebCore::HTMLAttachmentElement::parseAttribute):
* Source/WebCore/html/HTMLAttachmentElement.h:
* Source/WebCore/html/shadow/attachmentElementShadow.css:
(div#attachment-progress):

Canonical link: https://commits.webkit.org/262758@main
@webkit-commit-queue
Copy link
Collaborator

Committed 262758@main (31c3e79): https://commits.webkit.org/262758@main

Reviewed commits have been landed. Closing PR #12424 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 31c3e79 into WebKit:main Apr 9, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Apr 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HTML Editing For bugs in HTML editing support (including designMode and contentEditable).
Projects
None yet
6 participants