Summary: | IsolatedObject support for ProgressIndicator and Meter. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andres Gonzalez <andresg_22> | ||||||
Component: | New Bugs | Assignee: | Andres Gonzalez <andresg_22> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | aboxhall, apinheiro, cfleizach, commit-queue, darin, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Andres Gonzalez
2020-02-24 20:02:07 PST
Created attachment 391621 [details]
Patch
Comment on attachment 391621 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391621&action=review > Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:97 > + if (!gaugeRegionValue.isEmpty()) { > + StringBuilder builder; > + builder.append(description); > + if (builder.length()) > + builder.appendLiteral(", "); > + builder.append(gaugeRegionValue); > + description = builder.toString(); > + } Here’s a more efficient way to write this: if (!gaugeRegionValue.isEmpty()) description = makeString(description, description.isEmpty() ? "" : ", ", gaugeRegionValue); Or we could use the + operator if you prefer that to makeString. if (!gaugeRegionValue.isEmpty()) description = description + (description.isEmpty() ? "" : ", ") + gaugeRegionValue; Created attachment 391644 [details]
Patch
(In reply to Darin Adler from comment #2) > Comment on attachment 391621 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=391621&action=review > > > Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:97 > > + if (!gaugeRegionValue.isEmpty()) { > > + StringBuilder builder; > > + builder.append(description); > > + if (builder.length()) > > + builder.appendLiteral(", "); > > + builder.append(gaugeRegionValue); > > + description = builder.toString(); > > + } > > Here’s a more efficient way to write this: > > if (!gaugeRegionValue.isEmpty()) > description = makeString(description, description.isEmpty() ? "" : > ", ", gaugeRegionValue); > > Or we could use the + operator if you prefer that to makeString. > > if (!gaugeRegionValue.isEmpty()) > description = description + (description.isEmpty() ? "" : ", ") + > gaugeRegionValue; Thanks very much, Darin. Changed it to: if (!gaugeRegionValue.isEmpty()) description = description.isEmpty() ? gaugeRegionValue : description + ", " + gaugeRegionValue; So that we don't call operator+ unnecessarily when description.isEmpty. I assumed that the StringBuilder implementation was more efficient than the String concatenation and so I kept it. Good to know that that is not the case. Thanks again. Comment on attachment 391644 [details] Patch Clearing flags on attachment: 391644 Committed r257356: <https://trac.webkit.org/changeset/257356> All reviewed patches have been landed. Closing bug. |