Summary: | Rename Node::getRect/getPixelSnappedRect and remove ContainerNode::getRect | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Emil A Eklund <eae> | ||||||||||||||
Component: | DOM | Assignee: | Emil A Eklund <eae> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | cmarcelo, darin, dglazkov, eric, fpizlo, gustavo, gyuyoung.kim, hyatt, kling, mifenton, mitz, philn, rakuco, tkent, tonikitoo, webkit.review.bot, xan.lopez | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | 81016, 96226 | ||||||||||||||||
Bug Blocks: | |||||||||||||||||
Attachments: |
|
Description
Emil A Eklund
2012-03-16 14:53:46 PDT
Created attachment 132704 [details]
Patch for landing
Added math.h include to FractionalLayoutUnit.h, should make the gtk bot happy. If it does I'll set cq+.
(In reply to comment #1) > Created an attachment (id=132704) [details] > Patch for landing > > Added math.h include to FractionalLayoutUnit.h, should make the gtk bot happy. If it does I'll set cq+. Wrong bug, please ignore patch. Created attachment 159753 [details]
Patch
Created attachment 159765 [details]
Patch
Comment on attachment 159765 [details]
Patch
Why BoxRect in these names instead of just Box?
(In reply to comment #5) > (From update of attachment 159765 [details]) > Why BoxRect in these names instead of just Box? Trying to be consistent with existing methods that return a bounding box. Most methods we have that return a bounding box are named BoxRect, such as contentBoxRect, clientBoxRect, borderBoxRectInRegion and absoluteBoundingBoxRect. I'd be more than happy to rename this to just Box though if you think that is a better name. I certainly prefer it. I think the relevant question is whether there is more than one type that could be used to return a box. For example, maybe there’s a quad and a rect, and the rect version is lossy. If there is not more than one type, then we should s/BoxRect/Box/g. I'd want to ask Hyatt, I guess, in case he knows something. That makes sense. As these are the only versions in Node I'll rename them. Thanks! Created attachment 159775 [details]
Patch
Comment on attachment 159775 [details] Patch Attachment 159775 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13559061 Created attachment 159821 [details]
Patch
Darin/Hyatt any further comments on this? Comment on attachment 159821 [details]
Patch
r=me
Comment on attachment 159821 [details] Patch Rejecting attachment 159821 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ed at 1608 (offset 3 lines). patching file Source/WebKit/qt/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebKit/qt/Api/qwebelement.cpp Hunk #1 succeeded at 546 (offset 1 line). Hunk #2 succeeded at 1454 (offset 1 line). patching file Source/WebKit/qt/Api/qwebpage.cpp Hunk #1 succeeded at 1577 (offset -8 lines). Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'David Hyatt']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/13774925 Created attachment 162926 [details]
Patch for landing
Comment on attachment 162926 [details] Patch for landing Clearing flags on attachment: 162926 Committed r128006: <http://trac.webkit.org/changeset/128006> All reviewed patches have been landed. Closing bug. (In reply to comment #17) > All reviewed patches have been landed. Closing bug. I suspect this broke our MountainLion build: http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20%28Build%29/builds/1003/steps/compile-webkit/logs/stdio (In reply to comment #16) > (From update of attachment 162926 [details]) > Clearing flags on attachment: 162926 > > Committed r128006: <http://trac.webkit.org/changeset/128006> This broke the build because SVGElement has its own non-virtual boundingBox. See for example <http://build.webkit.org/builders/Apple%20Lion%20Release%20%28Build%29/builds/3594/steps/compile-webkit/logs/stdio/text>. (In reply to comment #19) > (In reply to comment #16) > > (From update of attachment 162926 [details] [details]) > > Clearing flags on attachment: 162926 > > > > Committed r128006: <http://trac.webkit.org/changeset/128006> > > This broke the build because SVGElement has its own non-virtual boundingBox. See for example <http://build.webkit.org/builders/Apple%20Lion%20Release%20%28Build%29/builds/3594/steps/compile-webkit/logs/stdio/text>. I tried to fix this in <http://trac.webkit.org/r128009>. (In reply to comment #20) > (In reply to comment #19) > > (In reply to comment #16) > > > (From update of attachment 162926 [details] [details] [details]) > > > Clearing flags on attachment: 162926 > > > > > > Committed r128006: <http://trac.webkit.org/changeset/128006> > > > > This broke the build because SVGElement has its own non-virtual boundingBox. See for example <http://build.webkit.org/builders/Apple%20Lion%20Release%20%28Build%29/builds/3594/steps/compile-webkit/logs/stdio/text>. > > I tried to fix this in <http://trac.webkit.org/r128009>. Was just about to commit a similar fix myself, thanks! (In reply to comment #16) > (From update of attachment 162926 [details]) > Clearing flags on attachment: 162926 > > Committed r128006: <http://trac.webkit.org/changeset/128006> This caused three spatial navigation tests to fail. See for example <http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20(Tests)/r128010%20(2654)/results.html>. (In reply to comment #22) > (In reply to comment #16) > > (From update of attachment 162926 [details] [details]) > > Clearing flags on attachment: 162926 > > > > Committed r128006: <http://trac.webkit.org/changeset/128006> > > This caused three spatial navigation tests to fail. See for example <http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20(Tests)/r128010%20(2654)/results.html>. Filed bug 96226. (In reply to comment #22) > (In reply to comment #16) > > (From update of attachment 162926 [details] [details]) > > Clearing flags on attachment: 162926 > > > > Committed r128006: <http://trac.webkit.org/changeset/128006> > > This caused three spatial navigation tests to fail. See for example <http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20(Tests)/r128010%20(2654)/results.html>. If this is a rename, as the title says, it should really not fail any test, and even, leave the test skipped is not a nice solution. Emil, do you have plans to investigate it? I'm looking into it at the moment, it is being tracked by 96226. |