ClientRect properties should be on the prototype. They are currently on the instance in WebKit. They are on the prototype in Firefox and Chrome.
Created attachment 284499 [details] Patch
Comment on attachment 284499 [details] Patch r=me
Comment on attachment 284499 [details] Patch The inspector tests look like regressions to me. What was valid numbers about the highlight have been lost.
(In reply to comment #3) > Comment on attachment 284499 [details] > Patch > > The inspector tests look like regressions to me. What was valid numbers > about the highlight have been lost. Joe, As I mention in the ChangeLog, those tests are dumping JSON.stringify(clientRect). This returns "{}" in Firefox and Chrome as well.
Oh, it looks like there are a few more tests failing on the bots: Regressions: Unexpected text-only failures (3) media/controls/elementOrder.html [ Failure ] media/controls/fullscreen-button-inline-layout.html [ Failure ] media/controls/statusDisplayBad.html [ Failure ] I'll investigate
Comment on attachment 284499 [details] Patch Attachment 284499 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1752151 New failing tests: media/controls/fullscreen-button-inline-layout.html media/controls/statusDisplayBad.html media/controls/elementOrder.html
Created attachment 284507 [details] Archive of layout-test-results from ews102 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 284499 [details] Patch Attachment 284499 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1752148 New failing tests: media/controls/fullscreen-button-inline-layout.html media/controls/statusDisplayBad.html media/controls/elementOrder.html
Created attachment 284508 [details] Archive of layout-test-results from ews105 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 284499 [details] Patch Attachment 284499 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1752174 New failing tests: media/controls/fullscreen-button-inline-layout.html media/controls/statusDisplayBad.html media/controls/elementOrder.html
Created attachment 284510 [details] Archive of layout-test-results from ews116 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
(In reply to comment #4) > (In reply to comment #3) > > Comment on attachment 284499 [details] > > Patch > > > > The inspector tests look like regressions to me. What was valid numbers > > about the highlight have been lost. > > Joe, As I mention in the ChangeLog, those tests are dumping > JSON.stringify(clientRect). > This returns "{}" in Firefox and Chrome as well. Then it sounds like the tests should be changed to dump new values instead of what appears to be now be useless data.
Comment on attachment 284499 [details] Patch Improved patch coming. The specification says that ClientRect should have a JSON serializer: https://drafts.fxtf.org/geometry/Overview.html#domrectreadonly
Created attachment 284524 [details] Patch
Created attachment 284525 [details] Patch
Created attachment 284526 [details] Patch
Created attachment 284533 [details] Patch
Requesting review again because I made additional (non-minor) changes.
Comment on attachment 284533 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284533&action=review r=me > Source/WebCore/bindings/js/JSClientRectCustom.cpp:47 > + CodeBlock* codeBlock = state.codeBlock(); > + PutPropertySlot slot(object, codeBlock ? codeBlock->isStrictMode() : false); Can toJSON ever be called directly from a user JS function? > Source/WebCore/bindings/js/JSClientRectCustom.cpp:53 > + object.put(&state, Identifier::fromString(&vm, reinterpret_cast<const LChar*>("top"), strlen("top")), jsNumber(rect.top()), slot); > + object.put(&state, Identifier::fromString(&vm, reinterpret_cast<const LChar*>("right"), strlen("right")), jsNumber(rect.right()), slot); > + object.put(&state, Identifier::fromString(&vm, reinterpret_cast<const LChar*>("bottom"), strlen("bottom")), jsNumber(rect.bottom()), slot); > + object.put(&state, Identifier::fromString(&vm, reinterpret_cast<const LChar*>("left"), strlen("left")), jsNumber(rect.left()), slot); > + object.put(&state, Identifier::fromString(&vm, reinterpret_cast<const LChar*>("width"), strlen("width")), jsNumber(rect.width()), slot); > + object.put(&state, Identifier::fromString(&vm, reinterpret_cast<const LChar*>("height"), strlen("height")), jsNumber(rect.height()), slot); I think Andreas would say you should use ASCIILIteral here.
(In reply to comment #19) > Comment on attachment 284533 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=284533&action=review > > r=me > > > Source/WebCore/bindings/js/JSClientRectCustom.cpp:47 > > + CodeBlock* codeBlock = state.codeBlock(); > > + PutPropertySlot slot(object, codeBlock ? codeBlock->isStrictMode() : false); > > Can toJSON ever be called directly from a user JS function? Yes, as per WebIDL. Why? > > > Source/WebCore/bindings/js/JSClientRectCustom.cpp:53 > > + object.put(&state, Identifier::fromString(&vm, reinterpret_cast<const LChar*>("top"), strlen("top")), jsNumber(rect.top()), slot); > > + object.put(&state, Identifier::fromString(&vm, reinterpret_cast<const LChar*>("right"), strlen("right")), jsNumber(rect.right()), slot); > > + object.put(&state, Identifier::fromString(&vm, reinterpret_cast<const LChar*>("bottom"), strlen("bottom")), jsNumber(rect.bottom()), slot); > > + object.put(&state, Identifier::fromString(&vm, reinterpret_cast<const LChar*>("left"), strlen("left")), jsNumber(rect.left()), slot); > > + object.put(&state, Identifier::fromString(&vm, reinterpret_cast<const LChar*>("width"), strlen("width")), jsNumber(rect.width()), slot); > > + object.put(&state, Identifier::fromString(&vm, reinterpret_cast<const LChar*>("height"), strlen("height")), jsNumber(rect.height()), slot); > > I think Andreas would say you should use ASCIILIteral here. Ah Ok. Someone should probably update the bindings generator then because I copied the pattern from there.
Comment on attachment 284533 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284533&action=review >>> Source/WebCore/bindings/js/JSClientRectCustom.cpp:47 >>> + PutPropertySlot slot(object, codeBlock ? codeBlock->isStrictMode() : false); >> >> Can toJSON ever be called directly from a user JS function? > > Yes, as per WebIDL. Why? c.f. http://heycam.github.io/webidl/#es-serializer which says there should be a "toJSON" property on the object. I don't see any reason why client JS would not be able to call it directly.
Created attachment 284547 [details] Patch
Created attachment 284549 [details] Patch
Comment on attachment 284549 [details] Patch Clearing flags on attachment: 284549 Committed r203702: <http://trac.webkit.org/changeset/203702>
All reviewed patches have been landed. Closing bug.