rdar://86446810
Created attachment 451997 [details] Patch
Created attachment 452094 [details] Rebase on trunk
Created attachment 452098 [details] Fix Big Sur build
Comment on attachment 452098 [details] Fix Big Sur build View in context: https://bugs.webkit.org/attachment.cgi?id=452098&action=review > Source/WebCore/dom/mac/ImageControlsMac.cpp:144 > if (ImageControlsMac::isImageControlsButtonElement(node)) { > - auto shadowHost = node.shadowHost(); > + RefPtr shadowHost = node.shadowHost(); > if (!is<HTMLImageElement>(*shadowHost)) > return false; > if (auto* image = imageFromImageElementNode(*shadowHost)) { This mix of early return and nested if doesn’t seem logical. Either 3 nested if statements or 3 early returns would be better than 2 and 1.
Comment on attachment 452098 [details] Fix Big Sur build View in context: https://bugs.webkit.org/attachment.cgi?id=452098&action=review Thanks for the review! >> Source/WebCore/dom/mac/ImageControlsMac.cpp:144 >> if (auto* image = imageFromImageElementNode(*shadowHost)) { > > This mix of early return and nested if doesn’t seem logical. Either 3 nested if statements or 3 early returns would be better than 2 and 1. Good point! Replaced the `if (auto* image = …)` with an early return. (Also changed this to use the `dynamicDowncast<HTMLImageElement>(…)` idiom here instead of having separate `is<>/downcast<>` calls).
Comment on attachment 452098 [details] Fix Big Sur build View in context: https://bugs.webkit.org/attachment.cgi?id=452098&action=review >>> Source/WebCore/dom/mac/ImageControlsMac.cpp:144 >>> if (auto* image = imageFromImageElementNode(*shadowHost)) { >> >> This mix of early return and nested if doesn’t seem logical. Either 3 nested if statements or 3 early returns would be better than 2 and 1. > > Good point! Replaced the `if (auto* image = …)` with an early return. > > (Also changed this to use the `dynamicDowncast<HTMLImageElement>(…)` idiom here instead of having separate `is<>/downcast<>` calls). Even though I didn't say anything, I was hoping you would consider that. Making Chris Dumez’s day.
Created attachment 452121 [details] For landing
Committed r289882 (247320@main): <https://commits.webkit.org/247320@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 452121 [details].