Bug 75471 – [Qt] Add UI for JavaScript Alert dialog in the Qt MiniBrowser

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Alexander Færøy]

SSIA.

[Alexander Færøy]

Created attachment 120941 [details]
Patch

[Alexander Færøy]

Adding Tor Arne.

[Tor Arne Vestbø]

Comment on attachment 120941 [details]
Patch

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

Looks good, but a few comments before landing

> Tools/MiniBrowser/qt/qml/Dialog.qml:58
> +        clip: true

Do we need to clip it?

> Tools/MiniBrowser/qt/qml/Dialog.qml:64
> +            font.weight: Font.Bold

Width, and eliding, so that we don't have to clip?

> Tools/MiniBrowser/qt/qml/Dialog.qml:69
> +            anchors.centerIn: parent

Should this have a width and a wrap mode?

> Tools/MiniBrowser/qt/qml/Dialog.qml:76
> +            anchors.margins: 10
> +            anchors.bottom: staticContent.bottom
> +            anchors.horizontalCenter: staticContent.horizontalCenter

I prefer anchors {} when there are more than one property

> Tools/MiniBrowser/qt/qml/DialogButton.qml:37
> +    color: "#fbfbfb"

color: mouseArea.pressed ? ....

> Tools/MiniBrowser/qt/qml/DialogButton.qml:57
> +        onPressed: button.color = "#eaeaea"

You can remove this and use a binding (above)

[Michael Brüning]

I think we should also include the new qml files in the OTHER_FILES section in MiniBrowser.pro ...

[Alexander Færøy]

Created attachment 120944 [details]
Patch

[WebKit Review Bot]

Comment on attachment 120944 [details]
Patch

Clearing flags on attachment: 120944

Committed r103940: <http://trac.webkit.org/changeset/103940>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.