Issue 40370044: tweak for domado-special viewport-dependent tests (Closed)
|
Created:
12 years, 2 months ago by felix8a Modified:
12 years, 2 months ago CC:
google-caja-discuss_googlegroups.com Base URL:
http://google-caja.googlecode.com/svn/trunk/ Visibility:
Public. |
Descriptiondomado-special has a few tests that rely on the guest content being
larger than the guest viewport, and deleting the history-sniff
tests made the content sometimes smaller than the viewport.
So this change reduces the viewport size.
This fixes
https://code.google.com/p/google-caja/issues/detail?id=1874
Patch Set 1 #
Total comments: 3
MessagesTotal messages: 10
https://codereview.appspot.com/40370044/diff/1/tests/com/google/caja/plugin/t... File tests/com/google/caja/plugin/test-domado-special-guest.html (right): https://codereview.appspot.com/40370044/diff/1/tests/com/google/caja/plugin/t... tests/com/google/caja/plugin/test-domado-special-guest.html:305: <p>testResizing and testScrolling need a little extra content to work as Instead of shrinking the viewport (which makes things less convenient when debugging), style this element to be bigger.
Sign in to reply to this message.
https://codereview.appspot.com/40370044/diff/1/tests/com/google/caja/plugin/t... File tests/com/google/caja/plugin/test-domado-special-guest.html (right): https://codereview.appspot.com/40370044/diff/1/tests/com/google/caja/plugin/t... tests/com/google/caja/plugin/test-domado-special-guest.html:305: <p>testResizing and testScrolling need a little extra content to work as On 2013/12/11 00:54:33, kpreid2 wrote: > Instead of shrinking the viewport (which makes things less convenient when > debugging), style this element to be bigger. I'm not sure how that would help with debugging?
Sign in to reply to this message.
https://codereview.appspot.com/40370044/diff/1/tests/com/google/caja/plugin/t... File tests/com/google/caja/plugin/test-domado-special-guest.html (right): https://codereview.appspot.com/40370044/diff/1/tests/com/google/caja/plugin/t... tests/com/google/caja/plugin/test-domado-special-guest.html:305: <p>testResizing and testScrolling need a little extra content to work as On 2013/12/11 01:12:37, felix8a wrote: > On 2013/12/11 00:54:33, kpreid2 wrote: > > Instead of shrinking the viewport (which makes things less convenient when > > debugging), style this element to be bigger. > > I'm not sure how that would help with debugging? A smaller viewport is harder to look through. Seeing the visual output of the tests is useful for debugging.
Sign in to reply to this message.
On 2013/12/11 01:58:13, kpreid2 wrote: > https://codereview.appspot.com/40370044/diff/1/tests/com/google/caja/plugin/t... > File tests/com/google/caja/plugin/test-domado-special-guest.html (right): > > https://codereview.appspot.com/40370044/diff/1/tests/com/google/caja/plugin/t... > tests/com/google/caja/plugin/test-domado-special-guest.html:305: <p>testResizing > and testScrolling need a little extra content to work as > On 2013/12/11 01:12:37, felix8a wrote: > > On 2013/12/11 00:54:33, kpreid2 wrote: > > > Instead of shrinking the viewport (which makes things less convenient when > > > debugging), style this element to be bigger. > > > > I'm not sure how that would help with debugging? > > A smaller viewport is harder to look through. Seeing the visual output of the > tests is useful for debugging. Sorry, this fell out of my inbox. Is this a problem you've run into? The visual output seems quite easy to see when I run it. This size is large enough to see about a dozen test divs at a time.
Sign in to reply to this message.
On 2014/01/07 20:42:59, felix8a wrote: > Sorry, this fell out of my inbox. Is this a problem you've run into? The visual > output seems quite easy to see when I run it. This size is large enough to see > about a dozen test divs at a time. It's not really the specific size of the viewport that's at issue — it's that we have an existing mechanism to handle the problem of the content needing to be a certain amount larger than the viewport, and the mechanism is that <p> that I pointed out; thus that's what should be tweaked, not the viewport size. But I do also have memories of debugging these tests being annoying due to the viewport size — I don't recall the exact scenario.
Sign in to reply to this message.
On 2014/01/07 21:19:30, kpreid_google wrote: > On 2014/01/07 20:42:59, felix8a wrote: > > Sorry, this fell out of my inbox. Is this a problem you've run into? The > visual > > output seems quite easy to see when I run it. This size is large enough to see > > about a dozen test divs at a time. > > It's not really the specific size of the viewport that's at issue — it's that we > have an existing mechanism to handle the problem of the content needing to be a > certain amount larger than the viewport, and the mechanism is that <p> that I > pointed out; thus that's what should be tweaked, not the viewport size. > > But I do also have memories of debugging these tests being annoying due to the > viewport size — I don't recall the exact scenario. Making the <p> larger seems like a worse option to me. How do I know how large the <p> should be, maybe 10000px is enough? Will a relative size like 200% be based off the right measurement? And larger content also makes it harder to view the results, since now I have to scroll an arbitrary distance to see the next result.
Sign in to reply to this message.
On 2014/01/07 21:24:59, felix8a wrote: > On 2014/01/07 21:19:30, kpreid_google wrote: > > It's not really the specific size of the viewport that's at issue — it's that we > > have an existing mechanism to handle the problem of the content needing to be a > > certain amount larger than the viewport, and the mechanism is that <p> that I > > pointed out; thus that's what should be tweaked, not the viewport size. > > > > But I do also have memories of debugging these tests being annoying due to the > > viewport size — I don't recall the exact scenario. > > Making the <p> larger seems like a worse option to me. How do I know how large > the <p> should be, maybe 10000px is enough? Will a relative size like 200% be > based off the right measurement? And larger content also makes it harder to view > the results, since now I have to scroll an arbitrary distance to see the next > result. I don't follow your argument. For the purpose of executing the test, making the <p> larger by some amount is exactly the same in effect as making the viewport smaller by the same amount. And the amount of scrolling you have to do is the same in either case (though what I propose does mean introducing more 'blank' space). I think the conclusion here is that we really ought to put the resizing/scrolling tests in their own dedicated file (like is already done for test-container-overflow) so their special requirements don't interfere with usability of other things. This change LGTM as is, doesn't matter that much.
Sign in to reply to this message.
On 2014/01/07 21:32:33, kpreid2 wrote: > I think the conclusion here is that we really ought to put the > resizing/scrolling tests in their own dedicated file (like is already done for > test-container-overflow) so their special requirements don't interfere with > usability of other things. ok I'll put a TODO for that
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
