Bug 173062 – Laili restaurant menu page does not display full menu in Safari (works in Chrome and Firefox)

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Dave Hyatt]

Laili restaurant menu page does not display full menu in Safari (works in Chrome and Firefox).

[Dave Hyatt]

Created attachment 312193 [details]
Patch

[Build Bot]

Attachment 312193 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderBox.h:671:  The parameter name "sizeType" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Comment on attachment 312193 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=312193&action=review

> Source/WebCore/ChangeLog:9
> +        Added fast/replaced/max-height-percent-inside-auto-block.html

Usually this is like:
        Test: fast/replaced/max-height-percent-inside-auto-block.html

and comes after the comment. prepare-ChangeLog does this for you.

> Source/WebCore/rendering/RenderBox.cpp:3155
> +    if (!replacedMinMaxLogicalHeightComputesAsNone(MinSize))
> +        minLogicalHeight = computeReplacedLogicalHeightUsing(MinSize, style().logicalMinHeight());
> +    LayoutUnit maxLogicalHeight = logicalHeight;
> +    if (!replacedMinMaxLogicalHeightComputesAsNone(MaxSize))
> +        maxLogicalHeight = computeReplacedLogicalHeightUsing(MaxSize, style().logicalMaxHeight());

Seems like we'll call containingBlockForAutoHeightDetection() twice via this coe and usually find the same containingBlock? Can we optimize to call it only once?

> Source/WebCore/rendering/RenderBox.h:671
> +    bool replacedMinMaxLogicalHeightComputesAsNone(SizeType sizeType) const;

You can remove the parameter name (style bot will probably tell you).

[Build Bot]

Comment on attachment 312193 [details]
Patch

Attachment 312193 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3888683

New failing tests:
fast/multicol/pagination/RightToLeft-max-width.html
fast/hidpi/percent-height-image-nested.html

[Build Bot]

Created attachment 312199 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 312193 [details]
Patch

Attachment 312193 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3888643

New failing tests:
fast/multicol/pagination/RightToLeft-max-width.html
fast/hidpi/percent-height-image-nested.html

[Build Bot]

Created attachment 312200 [details]
Archive of layout-test-results from ews112 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 312193 [details]
Patch

Attachment 312193 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3888687

New failing tests:
fast/multicol/pagination/RightToLeft-max-width.html
fast/hidpi/percent-height-image-nested.html

[Build Bot]

Created attachment 312201 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 312193 [details]
Patch

Attachment 312193 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3888725

New failing tests:
fast/multicol/pagination/RightToLeft-max-width.html

[Build Bot]

Created attachment 312203 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Dave Hyatt]

Created attachment 312216 [details]
Patch

[Dave Hyatt]

Created attachment 312217 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 312217 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=312217&action=review

> Source/WebCore/ChangeLog:11
> +        properly. min-max-height were not handled properly though and need to do the same check.

Say something about iBooks here?

Can we test the iBooks behavior?

> Source/WebCore/rendering/RenderBox.cpp:3130
> +static bool isRequestFromIBooks()

This should be called useReplacePercentageHeightAutoContainerQuirk() or something.

There should be a comment with a radar number, for a radar on iBooks that tracks their fixing the content to not need this quirk in future.

> Source/WebCore/rendering/RenderBox.cpp:3155
> +        return isRequestFromIBooks() ? false : cb->hasAutoHeightOrContainingBlockWithAutoHeight();

You can return false before this block for iBooks, since both return values are false here and below.

[Build Bot]

Comment on attachment 312217 [details]
Patch

Attachment 312217 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3889714

New failing tests:
fast/multicol/pagination/RightToLeft-max-width.html

[Build Bot]

Created attachment 312234 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Dave Hyatt]

Fixed in r217943. I converted the column failure into a reftest.

[Darin Adler]

Comment on attachment 312217 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=312217&action=review

>> Source/WebCore/rendering/RenderBox.cpp:3130
>> +static bool isRequestFromIBooks()
> 
> This should be called useReplacePercentageHeightAutoContainerQuirk() or something.
> 
> There should be a comment with a radar number, for a radar on iBooks that tracks their fixing the content to not need this quirk in future.

I would have stuck the inline keyword on this function.

[Karl Dubost]

Adding the information about this initial bug here. 

http://lailirestaurant.com/menu/ fails to show the full menu in Safari.

1. Load http://lailirestaurant.com/menu/
2. Scroll down
3. There's nothing below Salads in Safari. Chrome shows Wraps and Flatbreads

To assess if it is still needed.