refactor(core): drop confirm_blob_with_optional_pagination

Commit c300576d6c introduced `confirm_blob_with_optional_pagination` which proved to be unpopular and impractical. This commit brings back the old behaviour of having the `ask_pagination` parameter on `confirm_blob`. It also reverts back to using the old way of paginating `confirm_blob` on model R, which the aforementioned commit ignored and re-implemented from scratch. [no changelog]
2024-11-25 00:48:19 +00:00 · 2024-11-12 17:39:12 +01:00 · 2024-11-12 17:39:12 +01:00 · 3b40950f7a
commit 3b40950f7a
parent 38f0443000
7 changed files with 133 additions and 227 deletions
--- a/core/embed/rust/src/ui/model_tr/component/page.rs
+++ b/core/embed/rust/src/ui/model_tr/component/page.rs
@ -2,7 +2,7 @@ use crate::{
    translations::TR,
    ui::{
        component::{Child, Component, ComponentExt, Event, EventCtx, Pad, PageMsg, Paginate},
-        display::{Color, Font},
+        display::Color,
        geometry::{Insets, Rect},
        shape::Renderer,
    },
@ -13,25 +13,16 @@ use super::{
    ButtonDetails, ButtonLayout, ButtonPos,
 };

-#[derive(PartialEq)]
-enum LastPageLayout {
-    Confirm,
-    ArmedConfirmPlusInfo,
-}
-
 pub struct ButtonPage<T>
 where
    T: Component + Paginate,
 {
    page_count: usize,
    active_page: usize,
-    page_limit: Option<usize>,
-    last_page_layout: LastPageLayout,
    content: Child<T>,
    pad: Pad,
    cancel_btn_details: Option<ButtonDetails>,
    confirm_btn_details: Option<ButtonDetails>,
-    info_btn_details: Option<ButtonDetails>,
    back_btn_details: Option<ButtonDetails>,
    next_btn_details: Option<ButtonDetails>,
    buttons: Child<ButtonController>,
@ -45,17 +36,10 @@ where
        Self {
            page_count: 0, // will be set in place()
            active_page: 0,
-            page_limit: None,
-            last_page_layout: LastPageLayout::Confirm,
            content: Child::new(content),
            pad: Pad::with_background(background).with_clear(),
            cancel_btn_details: Some(ButtonDetails::cancel_icon()),
            confirm_btn_details: Some(ButtonDetails::text(TR::buttons__confirm.into())),
-            info_btn_details: Some(
-                ButtonDetails::text("i".into())
-                    .with_fixed_width(theme::BUTTON_ICON_WIDTH)
-                    .with_font(Font::NORMAL),
-            ),
            back_btn_details: Some(ButtonDetails::up_arrow_icon()),
            next_btn_details: Some(ButtonDetails::down_arrow_icon_wide()),
            // Setting empty layout for now, we do not yet know the page count.
@ -65,12 +49,6 @@ where
        }
    }

-    pub fn with_armed_confirm_plus_info(mut self) -> Self {
-        self.last_page_layout = LastPageLayout::ArmedConfirmPlusInfo;
-        self.confirm_btn_details = Some(ButtonDetails::armed_text(TR::words__confirm.into()));
-        self
-    }
-
    pub fn with_cancel_btn(mut self, btn_details: Option<ButtonDetails>) -> Self {
        self.cancel_btn_details = btn_details;
        self
@ -91,11 +69,6 @@ where
        self
    }

-    pub fn with_page_limit(mut self, page_limit: Option<usize>) -> Self {
-        self.page_limit = page_limit;
-        self
-    }
-
    pub fn has_next_page(&self) -> bool {
        self.active_page < self.page_count - 1
    }
@ -142,9 +115,8 @@ where

    fn get_button_layout(&self, has_prev: bool, has_next: bool) -> ButtonLayout {
        let btn_left = self.get_left_button_details(!has_prev);
-        let btn_middle = self.get_middle_button_details(has_next);
        let btn_right = self.get_right_button_details(has_next);
-        ButtonLayout::new(btn_left, btn_middle, btn_right)
+        ButtonLayout::new(btn_left, None, btn_right)
    }

    fn get_left_button_details(&self, is_first: bool) -> Option<ButtonDetails> {
@ -155,21 +127,11 @@ where
        }
    }

-    fn get_middle_button_details(&self, has_next_page: bool) -> Option<ButtonDetails> {
-        if has_next_page || self.last_page_layout == LastPageLayout::Confirm {
-            None
-        } else {
-            self.confirm_btn_details.clone()
-        }
-    }
-
    fn get_right_button_details(&self, has_next_page: bool) -> Option<ButtonDetails> {
        if has_next_page {
            self.next_btn_details.clone()
-        } else if self.last_page_layout == LastPageLayout::Confirm {
-            self.confirm_btn_details.clone()
        } else {
-            self.info_btn_details.clone()
+            self.confirm_btn_details.clone()
        }
    }
 }
@ -203,10 +165,6 @@ where
        // Need to be called here, only after content is placed
        // and we can calculate the page count.
        self.page_count = self.content.page_count();
-        if let Some(limit) = self.page_limit {
-            self.page_count = self.page_count.min(limit);
-        }
-
        self.set_buttons_for_initial_page(self.page_count);
        self.buttons.place(button_area);
        bounds
@ -226,25 +184,17 @@ where
                        return Some(PageMsg::Cancelled);
                    }
                }
-                ButtonPos::Middle => {
-                    return Some(PageMsg::Confirmed);
-                }
                ButtonPos::Right => {
                    if self.has_next_page() {
                        // Clicked NEXT. Scroll down.
                        self.go_to_next_page();
                        self.change_page(ctx);
                    } else {
-                        match self.last_page_layout {
-                            LastPageLayout::Confirm => {
+                        // Clicked CONFIRM. Send result.
                        return Some(PageMsg::Confirmed);
                    }
-                            LastPageLayout::ArmedConfirmPlusInfo => {
-                                return Some(PageMsg::Info);
-                            }
-                        }
-                    }
                }
+                _ => {}
            }
        }

--- a/core/embed/rust/src/ui/model_tr/layout.rs
+++ b/core/embed/rust/src/ui/model_tr/layout.rs
@ -95,7 +95,6 @@ where
        match msg {
            PageMsg::Confirmed => Ok(CONFIRMED.as_obj()),
            PageMsg::Cancelled => Ok(CANCELLED.as_obj()),
-            PageMsg::Info => Ok(INFO.as_obj()),
            _ => Err(Error::TypeError),
        }
    }
@ -245,9 +244,7 @@ fn content_in_button_page<T: Component + Paginate + MaybeTrace + 'static>(
    content: T,
    verb: TString<'static>,
    verb_cancel: Option<TString<'static>>,
-    info: bool,
    hold: bool,
-    page_limit: Option<usize>,
 ) -> Result<Obj, Error> {
    // Left button - icon, text or nothing.
    let cancel_btn = verb_cancel.map(ButtonDetails::from_text_possible_icon);
@ -263,15 +260,10 @@ fn content_in_button_page<T: Component + Paginate + MaybeTrace + 'static>(
        confirm_btn = confirm_btn.map(|btn| btn.with_default_duration());
    }

-    let mut content = ButtonPage::new(content, theme::BG)
+    let content = ButtonPage::new(content, theme::BG)
        .with_cancel_btn(cancel_btn)
-        .with_page_limit(page_limit)
        .with_confirm_btn(confirm_btn);

-    if info {
-        content = content.with_armed_confirm_plus_info();
-    }
-
    let mut frame = ScrollableFrame::new(content);
    if !title.is_empty() {
        frame = frame.with_title(title);
@ -312,7 +304,7 @@ extern "C" fn new_confirm_action(n_args: usize, args: *const Obj, kwargs: *mut M
            paragraphs.into_paragraphs()
        };

-        content_in_button_page(title, paragraphs, verb, verb_cancel, false, hold, None)
+        content_in_button_page(title, paragraphs, verb, verb_cancel, hold)
    };
    unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) }
 }
@ -335,13 +327,8 @@ extern "C" fn new_confirm_blob(n_args: usize, args: *const Obj, kwargs: *mut Map
            .get(Qstr::MP_QSTR_verb_cancel)
            .unwrap_or_else(|_| Obj::const_none())
            .try_into_option()?;
-        let info: bool = kwargs.get_or(Qstr::MP_QSTR_info, false)?;
        let hold: bool = kwargs.get_or(Qstr::MP_QSTR_hold, false)?;
        let chunkify: bool = kwargs.get_or(Qstr::MP_QSTR_chunkify, false)?;
-        let page_limit: Option<usize> = kwargs
-            .get(Qstr::MP_QSTR_page_limit)
-            .unwrap_or_else(|_| Obj::const_none())
-            .try_into_option()?;

        let style = if chunkify {
            // Chunkifying the address into smaller pieces when requested
@ -365,9 +352,7 @@ extern "C" fn new_confirm_blob(n_args: usize, args: *const Obj, kwargs: *mut Map
            paragraphs,
            verb.unwrap_or(TR::buttons__confirm.into()),
            verb_cancel,
-            info,
            hold,
-            page_limit,
        )
    };
    unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) }
@ -420,9 +405,7 @@ extern "C" fn new_confirm_properties(n_args: usize, args: *const Obj, kwargs: *m
            paragraphs.into_paragraphs(),
            button_text,
            Some("".into()),
-            false,
            hold,
-            None,
        )
    };
    unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) }
@ -452,15 +435,7 @@ extern "C" fn new_confirm_reset_device(n_args: usize, args: *const Obj, kwargs:
            .text_bold(TR::reset__tos_link);
        let formatted = FormattedText::new(ops).vertically_centered();

-        content_in_button_page(
-            title,
-            formatted,
-            button,
-            Some("".into()),
-            false,
-            false,
-            None,
-        )
+        content_in_button_page(title, formatted, button, Some("".into()), false)
    };
    unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) }
 }
@ -542,9 +517,7 @@ extern "C" fn new_confirm_value(n_args: usize, args: *const Obj, kwargs: *mut Ma
            paragraphs,
            verb.unwrap_or(TR::buttons__confirm.into()),
            Some("".into()),
-            false,
            hold,
-            None,
        )
    };
    unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) }
@ -567,9 +540,7 @@ extern "C" fn new_confirm_joint_total(n_args: usize, args: *const Obj, kwargs: *
            paragraphs,
            TR::buttons__hold_to_confirm.into(),
            Some("".into()),
-            false,
            true,
-            None,
        )
    };
    unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) }
@ -600,8 +571,6 @@ extern "C" fn new_confirm_modify_output(n_args: usize, args: *const Obj, kwargs:
            TR::buttons__confirm.into(),
            Some("".into()),
            false,
-            false,
-            None,
        )
    };
    unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) }
@ -975,8 +944,6 @@ extern "C" fn new_confirm_modify_fee(n_args: usize, args: *const Obj, kwargs: *m
            TR::buttons__confirm.into(),
            Some("".into()),
            false,
-            false,
-            None,
        )
    };
    unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) }
@ -1263,8 +1230,6 @@ extern "C" fn new_confirm_more(n_args: usize, args: *const Obj, kwargs: *mut Map
            button,
            Some("<".into()),
            false,
-            false,
-            None,
        )
    };
    unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) }
@ -1290,9 +1255,7 @@ extern "C" fn new_confirm_coinjoin(n_args: usize, args: *const Obj, kwargs: *mut
            paragraphs,
            TR::buttons__hold_to_confirm.into(),
            None,
-            false,
            true,
-            None,
        )
    };
    unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) }
@ -1488,8 +1451,6 @@ extern "C" fn new_confirm_recovery(n_args: usize, args: *const Obj, kwargs: *mut
            button,
            Some("".into()),
            false,
-            false,
-            None,
        )
    };
    unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) }
@ -1542,8 +1503,6 @@ extern "C" fn new_show_group_share_success(
            TR::buttons__continue.into(),
            None,
            false,
-            false,
-            None,
        )
    };
    unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) }
--- a/core/src/apps/ethereum/layout.py
+++ b/core/src/apps/ethereum/layout.py
@ -4,7 +4,6 @@ from trezor import TR, ui
 from trezor.enums import ButtonRequestType
 from trezor.ui.layouts import (
    confirm_blob,
-    confirm_blob_with_optional_pagination,
    confirm_ethereum_staking_tx,
    confirm_text,
    should_show_more,
@ -174,15 +173,9 @@ def require_confirm_address(address_bytes: bytes) -> Awaitable[None]:


 def require_confirm_other_data(data: bytes, data_total: int) -> Awaitable[None]:
-    return confirm_blob_with_optional_pagination(
-        "confirm_data",
-        TR.ethereum__title_input_data,
-        data,
-        subtitle=TR.ethereum__data_size_template.format(data_total),
-        verb=TR.buttons__confirm,
-        verb_cancel=TR.send__cancel_sign,
-        br_code=ButtonRequestType.SignTx,
-    )
+    from trezor.ui.layouts import confirm_other_data
+
+    return confirm_other_data(data, data_total)


 async def confirm_typed_data_final() -> None:
--- a/core/src/trezor/ui/layouts/common.py
+++ b/core/src/trezor/ui/layouts/common.py
@ -73,6 +73,7 @@ async def with_info(
    br_name: str,
    br_code: ButtonRequestType,
    repeat_button_request: bool = False,  # TODO this should eventually always be true
+    info_layout_can_confirm: bool = False,  # whether the info layout is allowed to confirm the whole flow
 ) -> None:
    first_br = br_name
    next_br = br_name if repeat_button_request else None
@ -95,7 +96,13 @@ async def with_info(
        if result is trezorui2.CONFIRMED:
            return
        elif result is trezorui2.INFO:
-            await interact(info_layout, next_br, br_code, raise_on_cancel=None)
+            info_result = await interact(
+                info_layout, next_br, br_code, raise_on_cancel=None
+            )
+            if info_layout_can_confirm and info_result is trezorui2.CONFIRMED:
+                return
+            else:
+                # no matter what the info layout returns, we always go back to the main layout
                continue
        else:
            raise RuntimeError  # unexpected result
--- a/core/src/trezor/ui/layouts/mercury/init.py
+++ b/core/src/trezor/ui/layouts/mercury/init.py
@ -460,16 +460,23 @@ async def should_show_more(
        raise ActionCancelled


-def confirm_blob_with_optional_pagination(
+def confirm_blob(
    br_name: str,
    title: str,
    data: bytes | str,
+    description: str | None = None,
+    text_mono: bool = True,
    subtitle: str | None = None,
    verb: str | None = None,
    verb_cancel: str | None = None,
+    info: bool = True,
+    hold: bool = False,
    br_code: ButtonRequestType = BR_CODE_OTHER,
+    ask_pagination: bool = False,
    chunkify: bool = False,
+    prompt_screen: bool = True,
 ) -> Awaitable[None]:
+    if ask_pagination:
        main_layout = trezorui2.confirm_blob(
            title=title,
            data=data,
@ -498,25 +505,14 @@ def confirm_blob_with_optional_pagination(
        )

        return with_info(
-        main_layout, info_layout, br_name, br_code, repeat_button_request=True
+            main_layout,
+            info_layout,
+            br_name,
+            br_code,
+            repeat_button_request=True,
+            info_layout_can_confirm=True,
        )
-
-
-def confirm_blob(
-    br_name: str,
-    title: str,
-    data: bytes | str,
-    description: str | None = None,
-    text_mono: bool = True,
-    subtitle: str | None = None,
-    verb: str | None = None,
-    verb_cancel: str | None = None,
-    info: bool = True,
-    hold: bool = False,
-    br_code: ButtonRequestType = BR_CODE_OTHER,
-    chunkify: bool = False,
-    prompt_screen: bool = True,
-) -> Awaitable[None]:
+    else:
        layout = trezorui2.confirm_blob(
            title=title,
            data=data,
@ -537,6 +533,21 @@ def confirm_blob(
        )


+if not utils.BITCOIN_ONLY:
+
+    def confirm_other_data(data: bytes, data_total: int) -> Awaitable[None]:
+        return confirm_blob(
+            "confirm_data",
+            TR.ethereum__title_input_data,
+            data,
+            subtitle=TR.ethereum__data_size_template.format(data_total),
+            verb=TR.buttons__confirm,
+            verb_cancel=TR.send__cancel_sign,
+            br_code=ButtonRequestType.SignTx,
+            ask_pagination=True,
+        )
+
+
 def confirm_address(
    title: str,
    address: str,
--- a/core/src/trezor/ui/layouts/tr/init.py
+++ b/core/src/trezor/ui/layouts/tr/init.py
@ -560,29 +560,6 @@ async def should_show_more(
        raise ActionCancelled


-def confirm_blob_with_optional_pagination(
-    br_name: str,
-    title: str,
-    data: bytes | str,
-    subtitle: str | None = None,
-    verb: str | None = None,
-    verb_cancel: str | None = None,
-    br_code: ButtonRequestType = BR_CODE_OTHER,
-    chunkify: bool = False,
-) -> Awaitable[None]:
-    return confirm_blob(
-        br_name,
-        title,
-        data,
-        subtitle=subtitle,
-        verb=verb,
-        verb_cancel="",
-        br_code=br_code,
-        chunkify=chunkify,
-        ask_pagination=True,
-    )
-
-
 def confirm_blob(
    br_name: str,
    title: str,
@ -595,9 +572,9 @@ def confirm_blob(
    info: bool = True,
    hold: bool = False,
    br_code: ButtonRequestType = BR_CODE_OTHER,
+    ask_pagination: bool = False,
    chunkify: bool = False,
    prompt_screen: bool = True,
-    ask_pagination: bool = False,
 ) -> Awaitable[None]:
    verb = verb or TR.buttons__confirm  # def_arg
    layout = trezorui2.confirm_blob(
@ -654,6 +631,21 @@ async def _confirm_ask_pagination(
    assert False


+if not utils.BITCOIN_ONLY:
+
+    def confirm_other_data(data: bytes, data_total: int) -> Awaitable[None]:
+        return confirm_blob(
+            "confirm_data",
+            TR.ethereum__title_input_data,
+            data,
+            subtitle=TR.ethereum__data_size_template.format(data_total),
+            verb=TR.buttons__confirm,
+            verb_cancel=None,
+            br_code=ButtonRequestType.SignTx,
+            ask_pagination=True,
+        )
+
+
 def confirm_address(
    title: str,
    address: str,
--- a/core/src/trezor/ui/layouts/tt/init.py
+++ b/core/src/trezor/ui/layouts/tt/init.py
@ -523,32 +523,6 @@ async def should_show_more(
        raise ActionCancelled


-def confirm_blob_with_optional_pagination(
-    br_name: str,
-    title: str,
-    data: bytes | str,
-    subtitle: str | None = None,
-    verb: str | None = None,
-    verb_cancel: str | None = None,
-    br_code: ButtonRequestType = BR_CODE_OTHER,
-    chunkify: bool = False,
-) -> Awaitable[None]:
-    verb = verb or TR.buttons__confirm  # def_arg
-    layout = trezorui2.confirm_blob(
-        title=title,
-        description=None,
-        data=data,
-        verb=verb,
-        verb_cancel=None,
-        chunkify=chunkify,
-    )
-
-    if layout.page_count() > 1:
-        return _confirm_ask_pagination(br_name, title, data, subtitle or "", br_code)
-    else:
-        return raise_if_not_confirmed(layout, br_name, br_code)
-
-
 async def _confirm_ask_pagination(
    br_name: str,
    title: str,
@ -598,6 +572,7 @@ def confirm_blob(
    info: bool = True,
    hold: bool = False,
    br_code: ButtonRequestType = BR_CODE_OTHER,
+    ask_pagination: bool = False,
    chunkify: bool = False,
    prompt_screen: bool = True,
 ) -> Awaitable[None]:
@ -613,6 +588,10 @@ def confirm_blob(
        chunkify=chunkify,
    )

+    if ask_pagination and layout.page_count() > 1:
+        assert not hold
+        return _confirm_ask_pagination(br_name, title, data, description or "", br_code)
+    else:
        return raise_if_not_confirmed(
            layout,
            br_name,
@ -620,6 +599,21 @@ def confirm_blob(
        )


+if not utils.BITCOIN_ONLY:
+
+    def confirm_other_data(data: bytes, data_total: int) -> Awaitable[None]:
+        return confirm_blob(
+            "confirm_data",
+            TR.ethereum__title_input_data,
+            data,
+            subtitle=TR.ethereum__data_size_template.format(data_total),
+            verb=TR.buttons__confirm,
+            verb_cancel=None,
+            br_code=ButtonRequestType.SignTx,
+            ask_pagination=True,
+        )
+
+
 def confirm_address(
    title: str,
    address: str,